cpio_utils: support compression of encrypted artifacts
diff mbox series

Message ID 1517488878-30307-1-git-send-email-achille.fouilleul@gadz.org
State Changes Requested
Headers show
Series
  • cpio_utils: support compression of encrypted artifacts
Related show

Commit Message

Achille Fouilleul Feb. 1, 2018, 12:41 p.m. UTC
Signed-off-by: Achille Fouilleul <achille.fouilleul@gadz.org>
---
 Makefile                  |   2 +-
 archival/Makefile         |   1 -
 archival/gun.c            | 519 ----------------------------------------------
 core/cpio_utils.c         | 338 ++++++++++++++++++++++--------
 doc/source/roadmap.rst    |   5 -
 handlers/ubivol_handler.c |   4 +
 include/util.h            |  18 --
 7 files changed, 255 insertions(+), 632 deletions(-)
 delete mode 100644 archival/Makefile
 delete mode 100644 archival/gun.c

Comments

Stefano Babic Feb. 4, 2018, 11:22 a.m. UTC | #1
Hi Achille,

On 01/02/2018 13:41, Achille Fouilleul wrote:
> Signed-off-by: Achille Fouilleul <achille.fouilleul@gadz.org>
> ---
>  Makefile                  |   2 +-
>  archival/Makefile         |   1 -
>  archival/gun.c            | 519 ----------------------------------------------
>  core/cpio_utils.c         | 338 ++++++++++++++++++++++--------
>  doc/source/roadmap.rst    |   5 -
>  handlers/ubivol_handler.c |   4 +
>  include/util.h            |  18 --
>  7 files changed, 255 insertions(+), 632 deletions(-)
>  delete mode 100644 archival/Makefile
>  delete mode 100644 archival/gun.c
> 

I appreciate your work cleaning up the code and dropping the old gun.c,
this was also one point in my TODO list. Some remarks:


> diff --git a/Makefile b/Makefile
> index 812a8a4..11fc2f5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -343,7 +343,7 @@ include $(srctree)/Makefile.flags
>  # Defaults to vmlinux, but the arch makefile usually adds further targets
>  
>  objs-y		:= core handlers
> -libs-y		:= archival corelib ipc mongoose parser suricatta bootloader
> +libs-y		:= corelib ipc mongoose parser suricatta bootloader
>  tools-y		:= tools
>  
>  swupdate-dirs	:= $(objs-y) $(libs-y)
> diff --git a/archival/Makefile b/archival/Makefile
> deleted file mode 100644
> index 5715d0b..0000000
> --- a/archival/Makefile
> +++ /dev/null
> @@ -1 +0,0 @@
> -lib-$(CONFIG_GUNZIP)		+= gun.o

Your match makes CONFIG_GUNZIP useless. HAVE_ZLIB is currently doing the
same job. Remove CONFIG_GUNZIP at all and use HAVE_ZLIB when necessary.

> diff --git a/archival/gun.c b/archival/gun.c
> deleted file mode 100644
> index ade0379..0000000
> --- a/archival/gun.c
> +++ /dev/null
> @@ -1,519 +0,0 @@
> -/* gun.c -- simple gunzip to give an example of the use of inflateBack()
> - * Copyright (C) 2003, 2005 Mark Adler
> - * For conditions of distribution and use, see copyright notice in zlib.h
> -   Version 1.3  12 June 2005  Mark Adler */
> -
> -/* Version history:
> -   1.0  16 Feb 2003  First version for testing of inflateBack()
> -   1.1  21 Feb 2005  Decompress concatenated gzip streams
> -                     Remove use of "this" variable (C++ keyword)
> -                     Fix return value for in()
> -                     Improve allocation failure checking
> -                     Add typecasting for void * structures
> -                     Add -h option for command version and usage
> -                     Add a bunch of comments
> -   1.2  20 Mar 2005  Add Unix compress (LZW) decompression
> -                     Copy file attributes from input file to output file
> -   1.3  12 Jun 2005  Add casts for error messages [Oberhumer]
> - */
> -
> -/* external functions and related types and constants */
> -#include <stdio.h>          /* fprintf() */
> -#include <stdlib.h>         /* malloc(), free() */
> -#include <string.h>         /* strerror(), strcmp(), strlen(), memcpy() */
> -#include <errno.h>          /* errno */
> -#include <fcntl.h>          /* open() */
> -#include <unistd.h>         /* read(), write(), close(), chown(), unlink() */
> -#include <sys/types.h>
> -#include <sys/stat.h>       /* stat(), chmod() */
> -#include <utime.h>          /* utime() */
> -#include <zlib.h>           /* inflateBackInit(), inflateBack(), */
> -                            /* inflateBackEnd(), crc32() */
> -#include <util.h>
> -#include <progress.h>
> -
> -/* buffer constants */
> -#define SIZE 32768U         /* input and output buffer sizes */
> -#define PIECE 16384         /* limits i/o chunks for 16-bit int case */
> -
> -#define MODULE_NAME "gunzip"
> -
> -/* structure for infback() to pass to input function in() -- it maintains the
> -   input file and a buffer of size SIZE */
> -struct ind {
> -    int infile;
> -    unsigned char *inbuf;
> -    unsigned long *offs;
> -    unsigned long *checksum;
> -    int nbytes;
> -    int total;
> -    int percent;
> -    void *dgst;
> -};
> -
> -/* Load input buffer, assumed to be empty, and return bytes loaded and a
> -   pointer to them.  read() is called until the buffer is full, or until it
> -   returns end-of-file or error.  Return 0 on error. */
> -static unsigned in(void *in_desc, unsigned char **buf)
> -{
> -    int ret;
> -    unsigned len;
> -    unsigned char *next;
> -    struct ind *me = (struct ind *)in_desc;
> -    unsigned int percent;
> -
> -    next = me->inbuf;
> -    *buf = next;
> -    len = 0;
> -    do {
> -        ret = PIECE;
> -        if ((unsigned)ret > SIZE - len)
> -            ret = (int)(SIZE - len);
> -	if (ret > me->nbytes)
> -		ret = me->nbytes;
> -	ret = fill_buffer(me->infile, next, ret, me->offs, (uint32_t *)me->checksum, me->dgst);
> -        if (ret < 0) {
> -            len = 0;
> -            break;
> -        }
> -        next += ret;
> -        len += ret;
> -	me->nbytes -= ret;
> -    } while (ret != 0 && len < SIZE);
> -    percent = (unsigned int)(((double)(me->total - me->nbytes)) * 100 /
> -		    (me->total ? me->total : 1));
> -    if (percent != (unsigned int)me->percent) {
> -	me->percent = percent;
> -	swupdate_progress_update(percent);
> -    }
> -    return len;
> -}
> -
> -/* structure for infback() to pass to output function out() -- it maintains the
> -   output file, a running CRC-32 check on the output and the total number of
> -   bytes output, both for checking against the gzip trailer.  (The length in
> -   the gzip trailer is stored modulo 2^32, so it's ok if a long is 32 bits and
> -   the output is greater than 4 GB.) */
> -struct outd {
> -    int outfile;
> -    int check;                  /* true if checking crc and total */
> -    unsigned long crc;
> -    unsigned long total;
> -};
> -
> -/* Write output buffer and update the CRC-32 and total bytes written.  write()
> -   is called until all of the output is written or an error is encountered.
> -   On success out() returns 0.  For a write failure, out() returns 1.  If the
> -   output file descriptor is -1, then nothing is written.
> - */
> -static int out(void *out_desc, unsigned char *buf, unsigned len)
> -{
> -    int ret;
> -    struct outd *me = (struct outd *)out_desc;
> -
> -    if (me->check) {
> -        me->crc = crc32(me->crc, buf, len);
> -        me->total += len;
> -    }
> -    if (me->outfile != -1)
> -        do {
> -            ret = PIECE;
> -            if ((unsigned)ret > len)
> -                ret = (int)len;
> -            ret = (int)write(me->outfile, buf, ret);
> -            if (ret == -1)
> -                return 1;
> -            buf += ret;
> -            len -= ret;
> -        } while (len != 0);
> -    return 0;
> -}
> -
> -/* next input byte macro for use inside lunpipe() and gunpipe() */
> -#define NEXT() (have ? 0 : (have = in(indp, &next)), \
> -                last = have ? (have--, (int)(*next++)) : -1)
> -
> -/* memory for gunpipe() and lunpipe() --
> -   the first 256 entries of prefix[] and suffix[] are never used, could
> -   have offset the index, but it's faster to waste the memory */
> -unsigned char inbuf[SIZE];              /* input buffer */
> -unsigned char outbuf[SIZE];             /* output buffer */
> -unsigned short prefix[65536];           /* index to LZW prefix string */
> -unsigned char suffix[65536];            /* one-character LZW suffix */
> -unsigned char match[65280 + 2];         /* buffer for reversed match or gzip
> -                                           32K sliding window */
> -
> -/* throw out what's left in the current bits byte buffer (this is a vestigial
> -   aspect of the compressed data format derived from an implementation that
> -   made use of a special VAX machine instruction!) */
> -#define FLUSHCODE() \
> -    do { \
> -        left = 0; \
> -        rem = 0; \
> -        if (chunk > have) { \
> -            chunk -= have; \
> -            have = 0; \
> -            if (NEXT() == -1) \
> -                break; \
> -            chunk--; \
> -            if (chunk > have) { \
> -                chunk = have = 0; \
> -                break; \
> -            } \
> -        } \
> -        have -= chunk; \
> -        next += chunk; \
> -        chunk = 0; \
> -    } while (0)
> -
> -/* Decompress a compress (LZW) file from indp to outfile.  The compress magic
> -   header (two bytes) has already been read and verified.  There are have bytes
> -   of buffered input at next.  strm is used for passing error information back
> -   to gunpipe().
> -
> -   lunpipe() will return Z_OK on success, Z_BUF_ERROR for an unexpected end of
> -   file, read error, or write error (a write error indicated by strm->next_in
> -   not equal to Z_NULL), or Z_DATA_ERROR for invalid input.
> - */
> -static int lunpipe(unsigned have, unsigned char *next, struct ind *indp,
> -                  int outfile, z_stream *strm)
> -{
> -    int last;                   /* last byte read by NEXT(), or -1 if EOF */
> -    unsigned chunk;             /* bytes left in current chunk */
> -    int left;                   /* bits left in rem */
> -    unsigned rem;               /* unused bits from input */
> -    int bits;                   /* current bits per code */
> -    unsigned code;              /* code, table traversal index */
> -    unsigned mask;              /* mask for current bits codes */
> -    int max;                    /* maximum bits per code for this stream */
> -    int flags;                  /* compress flags, then block compress flag */
> -    unsigned end;               /* last valid entry in prefix/suffix tables */
> -    unsigned temp;              /* current code */
> -    unsigned prev;              /* previous code */
> -    unsigned final;             /* last character written for previous code */
> -    unsigned stack;             /* next position for reversed string */
> -    unsigned outcnt;            /* bytes in output buffer */
> -    struct outd outd;           /* output structure */
> -
> -    /* set up output */
> -    outd.outfile = outfile;
> -    outd.check = 0;
> -
> -    /* process remainder of compress header -- a flags byte */
> -    flags = NEXT();
> -    if (last == -1)
> -        return Z_BUF_ERROR;
> -    if (flags & 0x60) {
> -        strm->msg = (char *)"unknown lzw flags set";
> -        return Z_DATA_ERROR;
> -    }
> -    max = flags & 0x1f;
> -    if (max < 9 || max > 16) {
> -        strm->msg = (char *)"lzw bits out of range";
> -        return Z_DATA_ERROR;
> -    }
> -    if (max == 9)                           /* 9 doesn't really mean 9 */
> -        max = 10;
> -    flags &= 0x80;                          /* true if block compress */
> -
> -    /* clear table */
> -    bits = 9;
> -    mask = 0x1ff;
> -    end = flags ? 256 : 255;
> -
> -    /* set up: get first 9-bit code, which is the first decompressed byte, but
> -       don't create a table entry until the next code */
> -    if (NEXT() == -1)                       /* no compressed data is ok */
> -        return Z_OK;
> -    final = prev = (unsigned)last;          /* low 8 bits of code */
> -    if (NEXT() == -1)                       /* missing a bit */
> -        return Z_BUF_ERROR;
> -    if (last & 1) {                         /* code must be < 256 */
> -        strm->msg = (char *)"invalid lzw code";
> -        return Z_DATA_ERROR;
> -    }
> -    rem = (unsigned)last >> 1;              /* remaining 7 bits */
> -    left = 7;
> -    chunk = bits - 2;                       /* 7 bytes left in this chunk */
> -    outbuf[0] = (unsigned char)final;       /* write first decompressed byte */
> -    outcnt = 1;
> -
> -    /* decode codes */
> -    stack = 0;
> -    for (;;) {
> -        /* if the table will be full after this, increment the code size */
> -        if (end >= mask && bits < max) {
> -            FLUSHCODE();
> -            bits++;
> -            mask <<= 1;
> -            mask++;
> -        }
> -
> -        /* get a code of length bits */
> -        if (chunk == 0)                     /* decrement chunk modulo bits */
> -            chunk = bits;
> -        code = rem;                         /* low bits of code */
> -        if (NEXT() == -1) {                 /* EOF is end of compressed data */
> -            /* write remaining buffered output */
> -            if (outcnt && out(&outd, outbuf, outcnt)) {
> -                strm->next_in = outbuf;     /* signal write error */
> -                return Z_BUF_ERROR;
> -            }
> -            return Z_OK;
> -        }
> -        code += (unsigned)last << left;     /* middle (or high) bits of code */
> -        left += 8;
> -        chunk--;
> -        if (bits > left) {                  /* need more bits */
> -            if (NEXT() == -1)               /* can't end in middle of code */
> -                return Z_BUF_ERROR;
> -            code += (unsigned)last << left; /* high bits of code */
> -            left += 8;
> -            chunk--;
> -        }
> -        code &= mask;                       /* mask to current code length */
> -        left -= bits;                       /* number of unused bits */
> -        rem = (unsigned)last >> (8 - left); /* unused bits from last byte */
> -
> -        /* process clear code (256) */
> -        if (code == 256 && flags) {
> -            FLUSHCODE();
> -            bits = 9;                       /* initialize bits and mask */
> -            mask = 0x1ff;
> -            end = 255;                      /* empty table */
> -            continue;                       /* get next code */
> -        }
> -
> -        /* special code to reuse last match */
> -        temp = code;                        /* save the current code */
> -        if (code > end) {
> -            /* Be picky on the allowed code here, and make sure that the code
> -               we drop through (prev) will be a valid index so that random
> -               input does not cause an exception.  The code != end + 1 check is
> -               empirically derived, and not checked in the original uncompress
> -               code.  If this ever causes a problem, that check could be safely
> -               removed.  Leaving this check in greatly improves gun's ability
> -               to detect random or corrupted input after a compress header.
> -               In any case, the prev > end check must be retained. */
> -            if (code != end + 1 || prev > end) {
> -                strm->msg = (char *)"invalid lzw code";
> -                return Z_DATA_ERROR;
> -            }
> -            match[stack++] = (unsigned char)final;
> -            code = prev;
> -        }
> -
> -        /* walk through linked list to generate output in reverse order */
> -        while (code >= 256) {
> -            match[stack++] = suffix[code];
> -            code = prefix[code];
> -        }
> -        match[stack++] = (unsigned char)code;
> -        final = code;
> -
> -        /* link new table entry */
> -        if (end < mask) {
> -            end++;
> -            prefix[end] = (unsigned short)prev;
> -            suffix[end] = (unsigned char)final;
> -        }
> -
> -        /* set previous code for next iteration */
> -        prev = temp;
> -
> -        /* write output in forward order */
> -        while (stack > SIZE - outcnt) {
> -            while (outcnt < SIZE)
> -                outbuf[outcnt++] = match[--stack];
> -            if (out(&outd, outbuf, outcnt)) {
> -                strm->next_in = outbuf; /* signal write error */
> -                return Z_BUF_ERROR;
> -            }
> -            outcnt = 0;
> -        }
> -        do {
> -            outbuf[outcnt++] = match[--stack];
> -        } while (stack);
> -
> -        /* loop for next code with final and prev as the last match, rem and
> -           left provide the first 0..7 bits of the next code, end is the last
> -           valid table entry */
> -    }
> -}
> -
> -
> -/* Decompress a gzip file from infile to outfile.  strm is assumed to have been
> -   successfully initialized with inflateBackInit().  The input file may consist
> -   of a series of gzip streams, in which case all of them will be decompressed
> -   to the output file.  If outfile is -1, then the gzip stream(s) integrity is
> -   checked and nothing is written.
> -
> -   The return value is a zlib error code: Z_MEM_ERROR if out of memory,
> -   Z_DATA_ERROR if the header or the compressed data is invalid, or if the
> -   trailer CRC-32 check or length doesn't match, Z_BUF_ERROR if the input ends
> -   prematurely or a write error occurs, or Z_ERRNO if junk (not a another gzip
> -   stream) follows a valid gzip stream.
> - */
> -static int gunpipe(z_stream *strm, int infile, unsigned long *offs, int nbytes, int outfile, uint32_t *checksum, void *dgst)
> -{
> -    int ret, first, last;
> -    unsigned have, flags, len;
> -    unsigned char *next;
> -    struct ind ind, *indp;
> -    struct outd outd;
> -
> -    /* setup input buffer */
> -    ind.infile = infile;
> -    ind.inbuf = inbuf;
> -    ind.nbytes = nbytes;
> -    ind.total = nbytes; /* Used just for progress */
> -    ind.percent = 0;
> -    ind.offs = offs;
> -    ind.checksum = (unsigned long *)checksum;
> -    ind.dgst = dgst; /* digest for computing hashes */
> -    indp = &ind;
> -
> -    /* decompress concatenated gzip streams */
> -    have = 0;                               /* no input data read in yet */
> -    first = 1;                              /* looking for first gzip header */
> -    strm->next_in = Z_NULL;                 /* so Z_BUF_ERROR means EOF */
> -    for (;;) {
> -        /* look for the two magic header bytes for a gzip stream */
> -        if (NEXT() == -1) {
> -            ret = Z_OK;
> -            break;                          /* empty gzip stream is ok */
> -        }
> -        if (last != 31 || (NEXT() != 139 && last != 157)) {
> -            strm->msg = (char *)"incorrect header check";
> -            ret = first ? Z_DATA_ERROR : Z_ERRNO;
> -            break;                          /* not a gzip or compress header */
> -        }
> -        first = 0;                          /* next non-header is junk */
> -
> -        /* process a compress (LZW) file -- can't be concatenated after this */
> -        if (last == 157) {
> -            ret = lunpipe(have, next, indp, outfile, strm);
> -            break;
> -        }
> -
> -        /* process remainder of gzip header */
> -        ret = Z_BUF_ERROR;
> -        if (NEXT() != 8) {                  /* only deflate method allowed */
> -            if (last == -1) break;
> -            strm->msg = (char *)"unknown compression method";
> -            ret = Z_DATA_ERROR;
> -            break;
> -        }
> -        flags = NEXT();                     /* header flags */
> -        NEXT();                             /* discard mod time, xflgs, os */
> -        NEXT();
> -        NEXT();
> -        NEXT();
> -        NEXT();
> -        NEXT();
> -        if (last == -1) break;
> -        if (flags & 0xe0) {
> -            strm->msg = (char *)"unknown header flags set";
> -            ret = Z_DATA_ERROR;
> -            break;
> -        }
> -        if (flags & 4) {                    /* extra field */
> -            len = NEXT();
> -            len += (unsigned)(NEXT()) << 8;
> -            if (last == -1) break;
> -            while (len > have) {
> -                len -= have;
> -                have = 0;
> -                if (NEXT() == -1) break;
> -                len--;
> -            }
> -            if (last == -1) break;
> -            have -= len;
> -            next += len;
> -        }
> -        if (flags & 8)                      /* file name */
> -            while (NEXT() != 0 && last != -1)
> -                ;
> -        if (flags & 16)                     /* comment */
> -            while (NEXT() != 0 && last != -1)
> -                ;
> -        if (flags & 2) {                    /* header crc */
> -            NEXT();
> -            NEXT();
> -        }
> -        if (last == -1) break;
> -
> -        /* set up output */
> -        outd.outfile = outfile;
> -        outd.check = 1;
> -        outd.crc = crc32(0L, Z_NULL, 0);
> -        outd.total = 0;
> -
> -        /* decompress data to output */
> -        strm->next_in = next;
> -        strm->avail_in = have;
> -        ret = inflateBack(strm, in, indp, out, &outd);
> -        if (ret != Z_STREAM_END) break;
> -        next = strm->next_in;
> -        have = strm->avail_in;
> -        strm->next_in = Z_NULL;             /* so Z_BUF_ERROR means EOF */
> -
> -        /* check trailer */
> -        ret = Z_BUF_ERROR;
> -        if (NEXT() != (int)(outd.crc & 0xff) ||
> -            NEXT() != (int)((outd.crc >> 8) & 0xff) ||
> -            NEXT() != (int)((outd.crc >> 16) & 0xff) ||
> -            NEXT() != (int)((outd.crc >> 24) & 0xff)) {
> -            /* crc error */
> -            if (last != -1) {
> -                strm->msg = (char *)"incorrect data check";
> -                ret = Z_DATA_ERROR;
> -            }
> -            break;
> -        }
> -        if (NEXT() != (int)(outd.total & 0xff) ||
> -            NEXT() != (int)((outd.total >> 8) & 0xff) ||
> -            NEXT() != (int)((outd.total >> 16) & 0xff) ||
> -            NEXT() != (int)((outd.total >> 24) & 0xff)) {
> -            /* length error */
> -            if (last != -1) {
> -                strm->msg = (char *)"incorrect length check";
> -                ret = Z_DATA_ERROR;
> -            }
> -            break;
> -        }
> -
> -        /* go back and look for another gzip stream */
> -    }
> -
> -    /* clean up and return */
> -    return ret;
> -}
> -
> -/* Process the gun command line arguments.  See the command syntax near the
> -   beginning of this source file. */
> -int decompress_image(int infile, unsigned long *offs, int nbytes,
> -	int outfile, uint32_t *checksum, void *dgst)
> -{
> -    int ret;
> -    unsigned char *window;
> -    z_stream strm;
> -
> -    *checksum = 0;
> -
> -    /* initialize inflateBack state for repeated use */
> -    window = match;                         /* reuse LZW match buffer */
> -    strm.zalloc = Z_NULL;
> -    strm.zfree = Z_NULL;
> -    strm.opaque = Z_NULL;
> -    ret = inflateBackInit(&strm, 15, window);
> -    if (ret != Z_OK) {
> -        ERROR("gun out of memory error--aborting\n");
> -        return 1;
> -    }
> -    errno = 0;
> -    ret = gunpipe(&strm, infile, offs, nbytes, outfile, checksum, dgst);
> -    /* clean up */
> -    inflateBackEnd(&strm);
> -    return ret;
> -}
> diff --git a/core/cpio_utils.c b/core/cpio_utils.c
> index 85f3fc0..26f155c 100644
> --- a/core/cpio_utils.c
> +++ b/core/cpio_utils.c
> @@ -5,10 +5,14 @@
>   * SPDX-License-Identifier:     GPL-2.0-or-later
>   */
>  
> +#include <stdbool.h>
>  #include <stdlib.h>
>  #include <stdio.h>
>  #include <unistd.h>
>  #include <errno.h>
> +#ifdef CONFIG_GUNZIP
> +#include <zlib.h>
> +#endif
>  
>  #include "generated/autoconf.h"
>  #include "cpiohdr.h"
> @@ -39,7 +43,7 @@ static int get_cpiohdr(unsigned char *buf, unsigned long *size,
>  	return 0;
>  }
>  
> -int fill_buffer(int fd, unsigned char *buf, unsigned int nbytes, unsigned long *offs,
> +static int fill_buffer(int fd, unsigned char *buf, unsigned int nbytes, unsigned long *offs,
>  	uint32_t *checksum, void *dgst)
>  {
>  	ssize_t len;
> @@ -99,72 +103,233 @@ static int copy_write(void *out, const void *buf, unsigned int len)
>  	return 0;
>  }
>  
> +typedef int (*PipelineStep)(void *state, void *buffer, size_t size);
> +
> +struct InputState
> +{
> +	int fdin;
> +	unsigned int nbytes;
> +	unsigned long *offs;
> +	void *dgst;	/* use a private context for HASH */
> +	uint32_t checksum;
> +};
> +
> +static int input_step(void *state, void *buffer, size_t size)
> +{
> +	struct InputState *s = (struct InputState *)state;
> +	if (size >= s->nbytes) {
> +		size = s->nbytes;
> +	}
> +	int ret = fill_buffer(s->fdin, buffer, size, s->offs, &s->checksum, s->dgst);
> +	if (ret < 0) {
> +		return ret;
> +	}
> +	s->nbytes -= size;
> +	return ret;
> +}
> +
> +struct DecryptState
> +{
> +	PipelineStep upstream_step;
> +	void *upstream_state;
> +
> +	void *dcrypt;	/* use a private context for decryption */
> +	uint8_t input[BUFF_SIZE];
> +	uint8_t output[BUFF_SIZE + AES_BLOCK_SIZE];
> +	int outlen;
> +	bool eof;
> +};

ok, you define several stages and you will set input / output of each
stage later. Got it.

> +
> +static int decrypt_step(void *state, void *buffer, size_t size)
> +{
> +	struct DecryptState *s = (struct DecryptState *)state;
> +	int ret;
> +	int inlen;
> +
> +	if (s->outlen != 0) {
> +		if (size > s->outlen) {
> +			size = s->outlen;
> +		}
> +		memcpy(buffer, s->output, size);
> +		s->outlen -= size;
> +		memmove(s->output, s->output + size, s->outlen);
> +		return size;
> +	}
> +
> +	ret = s->upstream_step(s->upstream_state, s->input, sizeof s->input);
> +	if (ret < 0) {
> +		return ret;
> +	}
> +
> +	inlen = ret;
> +
> +	if (!s->eof) {
> +		if (inlen != 0) {
> +			ret = swupdate_DECRYPT_update(s->dcrypt,
> +				s->output, &s->outlen, s->input, inlen);
> +		} else {
> +			/*
> +			 * Finalise the decryption. Further plaintext bytes may be written at
> +			 * this stage.
> +			 */
> +			ret = swupdate_DECRYPT_final(s->dcrypt,
> +				s->output, &s->outlen);
> +			s->eof = true;
> +		}
> +		if (ret < 0) {
> +			return ret;
> +		}
> +	}
> +
> +	if (s->outlen != 0) {
> +		if (size > s->outlen) {
> +			size = s->outlen;
> +		}
> +		memcpy(buffer, s->output, size);
> +		s->outlen -= size;
> +		memmove(s->output, s->output + size, s->outlen);
> +		return size;
> +	}
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_GUNZIP
> +
> +struct GunzipState
> +{
> +	PipelineStep upstream_step;
> +	void *upstream_state;
> +
> +	z_stream strm;
> +	bool initialized;
> +	uint8_t input[BUFF_SIZE];
> +	bool eof;
> +};
> +
> +static int gunzip_step(void *state, void *buffer, size_t size)
> +{
> +	struct GunzipState *s = (struct GunzipState *)state;
> +	int ret;
> +	int outlen = 0;
> +
> +	s->strm.next_out = buffer;
> +	s->strm.avail_out = size;
> +	while (outlen == 0) {
> +		if (s->strm.avail_in == 0) {
> +			ret = s->upstream_step(s->upstream_state, s->input, sizeof s->input);
> +			if (ret < 0) {
> +				return ret;
> +			}
> +			s->strm.avail_in = ret;
> +			s->strm.next_in = s->input;
> +		}
> +		if (s->eof) {
> +			break;
> +		}
> +
> +		ret = inflate(&s->strm, Z_NO_FLUSH);
> +		outlen = size - s->strm.avail_out;
> +		if (ret == Z_STREAM_END) {
> +			s->eof = true;
> +			break;
> +		}
> +		if (ret != Z_OK && ret != Z_BUF_ERROR) {
> +			ERROR("inflate failed (returned %d)", ret);
> +			return -1;
> +		}
> +	}
> +	return outlen;
> +}
> +
> +#endif
> +
>  int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs, unsigned long long seek,
>  	int skip_file, int __attribute__ ((__unused__)) compressed,
>  	uint32_t *checksum, unsigned char *hash, int encrypted, writeimage callback)
>  {
> -	unsigned long size;
> -	unsigned char *in = NULL, *inbuf;
> -	unsigned char *decbuf = NULL;
> -	unsigned long filesize = nbytes;
>  	unsigned int percent, prevpercent = 0;
>  	int ret = 0;
> -	void *dgst = NULL;	/* use a private context for HASH */
> -	void *dcrypt = NULL;	/* use a private context for decryption */
>  	int len;
>  	unsigned char md_value[64]; /*
>  				     *  Maximum hash is 64 bytes for SHA512
>  				     *  and we use sha256 in swupdate
>  				     */
>  	unsigned int md_len = 0;
> -	unsigned char *aes_key;
> -	unsigned char *ivt;
> +	unsigned char *aes_key = NULL;
> +	unsigned char *ivt = NULL;
>  	unsigned char *salt;
>  
> +	struct InputState input_state = {
> +		.fdin = fdin,
> +		.nbytes = nbytes,
> +		.offs = offs,
> +		.dgst = NULL,
> +		.checksum = 0
> +	};
> +
> +	struct DecryptState decrypt_state = {
> +		.upstream_step = NULL, .upstream_state = NULL,
> +		.dcrypt = NULL,
> +		.outlen = 0, .eof = false
> +	};
> +
> +#ifdef CONFIG_GUNZIP
> +	struct GunzipState gunzip_state = {
> +		.upstream_step = NULL, .upstream_state = NULL,
> +		.strm = {
> +			.zalloc = Z_NULL, .zfree = Z_NULL, .opaque = Z_NULL,
> +			.avail_in = 0, .next_in = Z_NULL,
> +			.avail_out = 0, .next_out = Z_NULL
> +		},
> +		.initialized = false,
> +		.eof = false
> +	};
> +#endif
> +
> +	PipelineStep step = NULL;
> +	void *state = NULL;
> +	uint8_t buffer[BUFF_SIZE];
> +
>  	if (!callback) {
>  		callback = copy_write;
>  	}
>  
> -	if (IsValidHash(hash)) {
> -		dgst = swupdate_HASH_init(SHA_DEFAULT);
> -		if (!dgst)
> -			return -EFAULT;
> -	}
> -
>  	if (checksum)
>  		*checksum = 0;
>  
> -	/*
> -	 * Simultaneous compression and decryption of images
> -	 * is not (yet ?) supported
> -	 */
> -	if (compressed && encrypted) {
> -		ERROR("encrypted zip images are not yet supported -- aborting\n");
> -		return -EINVAL;
> +	if (IsValidHash(hash)) {
> +		input_state.dgst = swupdate_HASH_init(SHA_DEFAULT);
> +		if (!input_state.dgst)
> +			return -EFAULT;
>  	}
> -
> -	in = (unsigned char *)malloc(BUFF_SIZE);
> -	if (!in)
> -		return -ENOMEM;
> -
> + 
>  	if (encrypted) {
> -		decbuf = (unsigned char *)calloc(1, BUFF_SIZE + AES_BLOCK_SIZE);
> -		if (!decbuf) {
> -			ret = -ENOMEM;
> -			goto copyfile_exit;
> -		}
> -
>  		aes_key = get_aes_key();
>  		ivt = get_aes_ivt();
>  		salt = get_aes_salt();
> -		dcrypt = swupdate_DECRYPT_init(aes_key, ivt, salt);
> -		if (!dcrypt) {
> +		decrypt_state.dcrypt = swupdate_DECRYPT_init(aes_key, ivt, salt);
> +		if (!decrypt_state.dcrypt) {
>  			ERROR("decrypt initialization failure, aborting");
>  			ret = -EFAULT;
>  			goto copyfile_exit;
>  		}
>  	}
>  
> +	if (compressed) {
> +#ifdef CONFIG_GUNZIP
> +		if (inflateInit2(&gunzip_state.strm, 16 + MAX_WBITS) != Z_OK) {

This is the core of the whole thing - thanks for the tip, I am not aware
of it. But it must be somehow documented without checking into Zlib
manual. Please add comments to indicate that initializing with +16 means
gunzip header.


> +			ERROR("inflateInit2 failed");
> +			ret = -EFAULT;
> +			goto copyfile_exit;
> +		}
> +		gunzip_state.initialized = true;
> +#else
> +		TRACE("Request decompressing, but CONFIG_GUNZIP not set !");
> +		return -1;
> +#endif
> +	}
> +
>  	if (seek) {
>  		int fdout = (out != NULL) ? *(int *)out : -1;
>  		TRACE("offset has been defined: %llu bytes\n", seek);
> @@ -175,74 +340,66 @@ int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs, unsi
>  		}
>  	}
>  
> -	int fdout = (out != NULL) ? *(int *)out : -1;
> +#ifdef CONFIG_GUNZIP
>  	if (compressed) {
> -		ret = decompress_image(fdin, offs, nbytes, fdout, checksum, dgst);
> -		if (ret < 0) {
> -			ERROR("gunzip failure %d (errno %d) -- aborting\n", ret, errno);
> -			goto copyfile_exit;
> +		if (encrypted) {
> +			decrypt_state.upstream_step = &input_step;
> +			decrypt_state.upstream_state = &input_state;
> +			gunzip_state.upstream_step = &decrypt_step;
> +			gunzip_state.upstream_state = &decrypt_state;
> +		} else {
> +			gunzip_state.upstream_step = &input_step;
> +			gunzip_state.upstream_state = &input_state;
>  		}
> +		step = &gunzip_step;
> +		state = &gunzip_state;
>  	} else {
> +#endif
> +		if (encrypted) {
> +			decrypt_state.upstream_step = &input_step;
> +			decrypt_state.upstream_state = &input_state;
> +			step = &decrypt_step;
> +			state = &decrypt_state;
> +		} else {
> +			step = &input_step;
> +			state = &input_state;
> +		}
> +#ifdef CONFIG_GUNZIP
> +	}
> +#endif
>  
> -	while (nbytes > 0) {
> -		size = (nbytes < BUFF_SIZE ? nbytes : BUFF_SIZE);
> -
> -		if ((ret = fill_buffer(fdin, in, size, offs, checksum, dgst) < 0)) {
> +	for (;;) {
> +		ret = step(state, buffer, sizeof buffer);
> +		if (ret < 0) {
>  			goto copyfile_exit;
>  		}
> -
> -		nbytes -= size;
> -		if (skip_file)
> +		if (ret == 0) {
> +			break;
> +		}
> +		if (skip_file) {
>  			continue;
> -
> -		inbuf = in;
> -		len = size;
> -
> -		if (encrypted) {
> -			ret = swupdate_DECRYPT_update(dcrypt, decbuf,
> -				&len, in, size);
> -			if (ret < 0)
> -				goto copyfile_exit;
> -			inbuf = decbuf;
>  		}
> -
> +		len = ret;
>  		/*
>  		 * If there is no enough place,
>  		 * returns an error and close the output file that
>  		 * results corrupted. This lets the cleanup routine
>  		 * to remove it
>  		 */
> -		if (callback(out, inbuf, len) < 0) {
> -			ret =-ENOSPC;
> +		if (callback(out, buffer, len) < 0) {
> +			ret = -ENOSPC;
>  			goto copyfile_exit;
>  		}
>  
> -		percent = (unsigned int)(((double)(filesize - nbytes)) * 100 / filesize);
> +		percent = (unsigned)(100ULL * (nbytes - input_state.nbytes) / nbytes);
>  		if (percent != prevpercent) {
>  			prevpercent = percent;
>  			swupdate_progress_update(percent);
>  		}
>  	}
>  
> -	}
> -
> -	/*
> -	 * Finalise the decryption. Further plaintext bytes may be written at
> -	 * this stage.
> -	 */
> -	if (encrypted) {
> -		ret = swupdate_DECRYPT_final(dcrypt, decbuf, &len);
> -		if (ret < 0)
> -			goto copyfile_exit;
> -		if (callback(out, decbuf, len) < 0) {
> -			ret =-ENOSPC;
> -			goto copyfile_exit;
> -		}
> -	}
> -
> -
>  	if (IsValidHash(hash)) {
> -		if (swupdate_HASH_final(dgst, md_value, &md_len) < 0) {
> +		if (swupdate_HASH_final(input_state.dgst, md_value, &md_len) < 0) {
>  			ret = -EFAULT;
>  			goto copyfile_exit;
>  		}
> @@ -266,21 +423,26 @@ int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs, unsi
>  		}
>  	}
>  
> -	fill_buffer(fdin, in, NPAD_BYTES(*offs), offs, checksum, NULL);
> +	fill_buffer(fdin, buffer, NPAD_BYTES(*offs), offs, checksum, NULL);
> +
> +	if (checksum != NULL) {
> +		*checksum = input_state.checksum;
> +	}
>  
>  	ret = 0;
>  
>  copyfile_exit:
> -	if (in)
> -		free(in);
> -	if (dcrypt) {
> -		swupdate_DECRYPT_cleanup(dcrypt);
> +	if (decrypt_state.dcrypt) {
> +		swupdate_DECRYPT_cleanup(decrypt_state.dcrypt);
> +	}
> +	if (input_state.dgst) {
> +		swupdate_HASH_cleanup(input_state.dgst);
>  	}
> -	if (decbuf)
> -		free(decbuf);
> -	if (dgst) {
> -		swupdate_HASH_cleanup(dgst);
> +#ifdef CONFIG_GUNZIP
> +	if (gunzip_state.initialized) {
> +		inflateEnd(&gunzip_state.strm);
>  	}
> +#endif
>  
>  	return ret;
>  }
> diff --git a/doc/source/roadmap.rst b/doc/source/roadmap.rst
> index 0502cbe..bc43b9f 100644
> --- a/doc/source/roadmap.rst
> +++ b/doc/source/roadmap.rst
> @@ -108,11 +108,6 @@ Current release supports verified images. That means that a handler
>  written in Lua could be now be part of the compound image, because
>  a unauthenticated handler cannot run.
>  
> -Encryption of compressed artifacts
> -==================================
> -
> -Currently, encrypted artifact are not compressed. Allow to compress artifacts before encryption.
> -
>  Support for evaluation boards
>  =============================
>  
> diff --git a/handlers/ubivol_handler.c b/handlers/ubivol_handler.c
> index 25ac837..2c4f95e 100644
> --- a/handlers/ubivol_handler.c
> +++ b/handlers/ubivol_handler.c
> @@ -19,6 +19,7 @@
>  #include "handler.h"
>  #include "flash.h"
>  #include "util.h"
> +#include "sslapi.h"
>  
>  void ubi_handler(void);
>  
> @@ -43,6 +44,9 @@ static int update_volume(libubi_t libubi, struct img_type *img,
>  	char sbuf[128];
>  
>  	bytes = img->size;
> +	if (img->is_encrypted) {
> +		bytes -= AES_BLOCK_SIZE;
> +	}

This is a different issue - it should be fixed in a separate patch.

>  
>  	if (!libubi) {
>  		ERROR("Request to write into UBI, but no UBI on system");
> diff --git a/include/util.h b/include/util.h
> index d43cd8c..bff9436 100644
> --- a/include/util.h
> +++ b/include/util.h
> @@ -130,8 +130,6 @@ char *sdup(const char *str);
>   */
>  typedef int (*writeimage) (void *out, const void *buf, unsigned int len);
>  
> -int fill_buffer(int fd, unsigned char *buf, unsigned int nbytes, unsigned long *offs,
> -	uint32_t *checksum, void *dgst);
>  int openfile(const char *filename);
>  int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs,
>  	unsigned long long seek,
> @@ -167,22 +165,6 @@ int get_install_info(sourcetype *source, char *buf, size_t len);
>  
>  unsigned long long ustrtoull(const char *cp, char **endp, unsigned int base);
>  
> -#ifdef CONFIG_GUNZIP
> -int decompress_image(int infile, unsigned long *offs, int nbytes,
> -	int outfile, uint32_t *checksum, void *dgst);
> -#else
> -static inline int decompress_image(int __attribute__ ((__unused__))infile,
> -		   unsigned long __attribute__ ((__unused__)) *offs,
> -		   int __attribute__ ((__unused__)) nbytes,
> -		   int __attribute__ ((__unused__)) outfile,
> -		   uint32_t __attribute__ ((__unused__)) *checksum,
> -		   void __attribute__ ((__unused__)) *dgst) {
> -
> -		TRACE("Request decompressing, but CONFIG_GUNZIP not set !");
> -	return -1;
> -}
> -#endif
> -
>  const char* get_tmpdir(void);
>  const char* get_tmpdirscripts(void);
>  
> 

My bigger concern is that the patch is replacing a central piece inside
SWUpdate. Can you ask which kind of tests you have already done and
which regressions tests ?

Thanks,
Stefano Babic
Achille Fouilleul Feb. 4, 2018, 6:28 p.m. UTC | #2
Hi Stefano,

Thanks for considering and reviewing my patch.

On Sun, Feb 4, 2018 at 12:22 PM, Stefano Babic <sbabic@denx.de> wrote:
>
> Hi Achille,
>
> On 01/02/2018 13:41, Achille Fouilleul wrote:
> > Signed-off-by: Achille Fouilleul <achille.fouilleul@gadz.org>
> > ---
> >  Makefile                  |   2 +-
> >  archival/Makefile         |   1 -
> >  archival/gun.c            | 519 ----------------------------------------------
> >  core/cpio_utils.c         | 338 ++++++++++++++++++++++--------
> >  doc/source/roadmap.rst    |   5 -
> >  handlers/ubivol_handler.c |   4 +
> >  include/util.h            |  18 --
> >  7 files changed, 255 insertions(+), 632 deletions(-)
> >  delete mode 100644 archival/Makefile
> >  delete mode 100644 archival/gun.c
> >
>
> I appreciate your work cleaning up the code and dropping the old gun.c,
> this was also one point in my TODO list.

Yes! Two birds, one stone: support one more option
(compression+encryption) with (hopefully) cleaner code.
>
> Some remarks:
>
>
> > diff --git a/Makefile b/Makefile
> > index 812a8a4..11fc2f5 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -343,7 +343,7 @@ include $(srctree)/Makefile.flags
> >  # Defaults to vmlinux, but the arch makefile usually adds further targets
> >
> >  objs-y               := core handlers
> > -libs-y               := archival corelib ipc mongoose parser suricatta bootloader
> > +libs-y               := corelib ipc mongoose parser suricatta bootloader
> >  tools-y              := tools
> >
> >  swupdate-dirs        := $(objs-y) $(libs-y)
> > diff --git a/archival/Makefile b/archival/Makefile
> > deleted file mode 100644
> > index 5715d0b..0000000
> > --- a/archival/Makefile
> > +++ /dev/null
> > @@ -1 +0,0 @@
> > -lib-$(CONFIG_GUNZIP)         += gun.o
>
> Your match makes CONFIG_GUNZIP useless. HAVE_ZLIB is currently doing the
> same job. Remove CONFIG_GUNZIP at all and use HAVE_ZLIB when necessary.

I can see a nuance between CONFIG_GUNZIP and HAVE_ZLIB: one may not
want to support Zlib compression (CONFIG_GUNZIP=n) even though they
have the library (HAVE_ZLIB=y). I thought that was the intent, this is
why I wrote the code that way.
>
>
> > diff --git a/archival/gun.c b/archival/gun.c
> > deleted file mode 100644
> > index ade0379..0000000
> > --- a/archival/gun.c
> > +++ /dev/null
> > @@ -1,519 +0,0 @@
> > -/* gun.c -- simple gunzip to give an example of the use of inflateBack()
> > - * Copyright (C) 2003, 2005 Mark Adler
> > - * For conditions of distribution and use, see copyright notice in zlib.h
> > -   Version 1.3  12 June 2005  Mark Adler */
> > -
> > -/* Version history:
> > -   1.0  16 Feb 2003  First version for testing of inflateBack()
> > -   1.1  21 Feb 2005  Decompress concatenated gzip streams
> > -                     Remove use of "this" variable (C++ keyword)
> > -                     Fix return value for in()
> > -                     Improve allocation failure checking
> > -                     Add typecasting for void * structures
> > -                     Add -h option for command version and usage
> > -                     Add a bunch of comments
> > -   1.2  20 Mar 2005  Add Unix compress (LZW) decompression
> > -                     Copy file attributes from input file to output file
> > -   1.3  12 Jun 2005  Add casts for error messages [Oberhumer]
> > - */
> > -
> > -/* external functions and related types and constants */
> > -#include <stdio.h>          /* fprintf() */
> > -#include <stdlib.h>         /* malloc(), free() */
> > -#include <string.h>         /* strerror(), strcmp(), strlen(), memcpy() */
> > -#include <errno.h>          /* errno */
> > -#include <fcntl.h>          /* open() */
> > -#include <unistd.h>         /* read(), write(), close(), chown(), unlink() */
> > -#include <sys/types.h>
> > -#include <sys/stat.h>       /* stat(), chmod() */
> > -#include <utime.h>          /* utime() */
> > -#include <zlib.h>           /* inflateBackInit(), inflateBack(), */
> > -                            /* inflateBackEnd(), crc32() */
> > -#include <util.h>
> > -#include <progress.h>
> > -
> > -/* buffer constants */
> > -#define SIZE 32768U         /* input and output buffer sizes */
> > -#define PIECE 16384         /* limits i/o chunks for 16-bit int case */
> > -
> > -#define MODULE_NAME "gunzip"
> > -
> > -/* structure for infback() to pass to input function in() -- it maintains the
> > -   input file and a buffer of size SIZE */
> > -struct ind {
> > -    int infile;
> > -    unsigned char *inbuf;
> > -    unsigned long *offs;
> > -    unsigned long *checksum;
> > -    int nbytes;
> > -    int total;
> > -    int percent;
> > -    void *dgst;
> > -};
> > -
> > -/* Load input buffer, assumed to be empty, and return bytes loaded and a
> > -   pointer to them.  read() is called until the buffer is full, or until it
> > -   returns end-of-file or error.  Return 0 on error. */
> > -static unsigned in(void *in_desc, unsigned char **buf)
> > -{
> > -    int ret;
> > -    unsigned len;
> > -    unsigned char *next;
> > -    struct ind *me = (struct ind *)in_desc;
> > -    unsigned int percent;
> > -
> > -    next = me->inbuf;
> > -    *buf = next;
> > -    len = 0;
> > -    do {
> > -        ret = PIECE;
> > -        if ((unsigned)ret > SIZE - len)
> > -            ret = (int)(SIZE - len);
> > -     if (ret > me->nbytes)
> > -             ret = me->nbytes;
> > -     ret = fill_buffer(me->infile, next, ret, me->offs, (uint32_t *)me->checksum, me->dgst);
> > -        if (ret < 0) {
> > -            len = 0;
> > -            break;
> > -        }
> > -        next += ret;
> > -        len += ret;
> > -     me->nbytes -= ret;
> > -    } while (ret != 0 && len < SIZE);
> > -    percent = (unsigned int)(((double)(me->total - me->nbytes)) * 100 /
> > -                 (me->total ? me->total : 1));
> > -    if (percent != (unsigned int)me->percent) {
> > -     me->percent = percent;
> > -     swupdate_progress_update(percent);
> > -    }
> > -    return len;
> > -}
> > -
> > -/* structure for infback() to pass to output function out() -- it maintains the
> > -   output file, a running CRC-32 check on the output and the total number of
> > -   bytes output, both for checking against the gzip trailer.  (The length in
> > -   the gzip trailer is stored modulo 2^32, so it's ok if a long is 32 bits and
> > -   the output is greater than 4 GB.) */
> > -struct outd {
> > -    int outfile;
> > -    int check;                  /* true if checking crc and total */
> > -    unsigned long crc;
> > -    unsigned long total;
> > -};
> > -
> > -/* Write output buffer and update the CRC-32 and total bytes written.  write()
> > -   is called until all of the output is written or an error is encountered.
> > -   On success out() returns 0.  For a write failure, out() returns 1.  If the
> > -   output file descriptor is -1, then nothing is written.
> > - */
> > -static int out(void *out_desc, unsigned char *buf, unsigned len)
> > -{
> > -    int ret;
> > -    struct outd *me = (struct outd *)out_desc;
> > -
> > -    if (me->check) {
> > -        me->crc = crc32(me->crc, buf, len);
> > -        me->total += len;
> > -    }
> > -    if (me->outfile != -1)
> > -        do {
> > -            ret = PIECE;
> > -            if ((unsigned)ret > len)
> > -                ret = (int)len;
> > -            ret = (int)write(me->outfile, buf, ret);
> > -            if (ret == -1)
> > -                return 1;
> > -            buf += ret;
> > -            len -= ret;
> > -        } while (len != 0);
> > -    return 0;
> > -}
> > -
> > -/* next input byte macro for use inside lunpipe() and gunpipe() */
> > -#define NEXT() (have ? 0 : (have = in(indp, &next)), \
> > -                last = have ? (have--, (int)(*next++)) : -1)
> > -
> > -/* memory for gunpipe() and lunpipe() --
> > -   the first 256 entries of prefix[] and suffix[] are never used, could
> > -   have offset the index, but it's faster to waste the memory */
> > -unsigned char inbuf[SIZE];              /* input buffer */
> > -unsigned char outbuf[SIZE];             /* output buffer */
> > -unsigned short prefix[65536];           /* index to LZW prefix string */
> > -unsigned char suffix[65536];            /* one-character LZW suffix */
> > -unsigned char match[65280 + 2];         /* buffer for reversed match or gzip
> > -                                           32K sliding window */
> > -
> > -/* throw out what's left in the current bits byte buffer (this is a vestigial
> > -   aspect of the compressed data format derived from an implementation that
> > -   made use of a special VAX machine instruction!) */
> > -#define FLUSHCODE() \
> > -    do { \
> > -        left = 0; \
> > -        rem = 0; \
> > -        if (chunk > have) { \
> > -            chunk -= have; \
> > -            have = 0; \
> > -            if (NEXT() == -1) \
> > -                break; \
> > -            chunk--; \
> > -            if (chunk > have) { \
> > -                chunk = have = 0; \
> > -                break; \
> > -            } \
> > -        } \
> > -        have -= chunk; \
> > -        next += chunk; \
> > -        chunk = 0; \
> > -    } while (0)
> > -
> > -/* Decompress a compress (LZW) file from indp to outfile.  The compress magic
> > -   header (two bytes) has already been read and verified.  There are have bytes
> > -   of buffered input at next.  strm is used for passing error information back
> > -   to gunpipe().
> > -
> > -   lunpipe() will return Z_OK on success, Z_BUF_ERROR for an unexpected end of
> > -   file, read error, or write error (a write error indicated by strm->next_in
> > -   not equal to Z_NULL), or Z_DATA_ERROR for invalid input.
> > - */
> > -static int lunpipe(unsigned have, unsigned char *next, struct ind *indp,
> > -                  int outfile, z_stream *strm)
> > -{
> > -    int last;                   /* last byte read by NEXT(), or -1 if EOF */
> > -    unsigned chunk;             /* bytes left in current chunk */
> > -    int left;                   /* bits left in rem */
> > -    unsigned rem;               /* unused bits from input */
> > -    int bits;                   /* current bits per code */
> > -    unsigned code;              /* code, table traversal index */
> > -    unsigned mask;              /* mask for current bits codes */
> > -    int max;                    /* maximum bits per code for this stream */
> > -    int flags;                  /* compress flags, then block compress flag */
> > -    unsigned end;               /* last valid entry in prefix/suffix tables */
> > -    unsigned temp;              /* current code */
> > -    unsigned prev;              /* previous code */
> > -    unsigned final;             /* last character written for previous code */
> > -    unsigned stack;             /* next position for reversed string */
> > -    unsigned outcnt;            /* bytes in output buffer */
> > -    struct outd outd;           /* output structure */
> > -
> > -    /* set up output */
> > -    outd.outfile = outfile;
> > -    outd.check = 0;
> > -
> > -    /* process remainder of compress header -- a flags byte */
> > -    flags = NEXT();
> > -    if (last == -1)
> > -        return Z_BUF_ERROR;
> > -    if (flags & 0x60) {
> > -        strm->msg = (char *)"unknown lzw flags set";
> > -        return Z_DATA_ERROR;
> > -    }
> > -    max = flags & 0x1f;
> > -    if (max < 9 || max > 16) {
> > -        strm->msg = (char *)"lzw bits out of range";
> > -        return Z_DATA_ERROR;
> > -    }
> > -    if (max == 9)                           /* 9 doesn't really mean 9 */
> > -        max = 10;
> > -    flags &= 0x80;                          /* true if block compress */
> > -
> > -    /* clear table */
> > -    bits = 9;
> > -    mask = 0x1ff;
> > -    end = flags ? 256 : 255;
> > -
> > -    /* set up: get first 9-bit code, which is the first decompressed byte, but
> > -       don't create a table entry until the next code */
> > -    if (NEXT() == -1)                       /* no compressed data is ok */
> > -        return Z_OK;
> > -    final = prev = (unsigned)last;          /* low 8 bits of code */
> > -    if (NEXT() == -1)                       /* missing a bit */
> > -        return Z_BUF_ERROR;
> > -    if (last & 1) {                         /* code must be < 256 */
> > -        strm->msg = (char *)"invalid lzw code";
> > -        return Z_DATA_ERROR;
> > -    }
> > -    rem = (unsigned)last >> 1;              /* remaining 7 bits */
> > -    left = 7;
> > -    chunk = bits - 2;                       /* 7 bytes left in this chunk */
> > -    outbuf[0] = (unsigned char)final;       /* write first decompressed byte */
> > -    outcnt = 1;
> > -
> > -    /* decode codes */
> > -    stack = 0;
> > -    for (;;) {
> > -        /* if the table will be full after this, increment the code size */
> > -        if (end >= mask && bits < max) {
> > -            FLUSHCODE();
> > -            bits++;
> > -            mask <<= 1;
> > -            mask++;
> > -        }
> > -
> > -        /* get a code of length bits */
> > -        if (chunk == 0)                     /* decrement chunk modulo bits */
> > -            chunk = bits;
> > -        code = rem;                         /* low bits of code */
> > -        if (NEXT() == -1) {                 /* EOF is end of compressed data */
> > -            /* write remaining buffered output */
> > -            if (outcnt && out(&outd, outbuf, outcnt)) {
> > -                strm->next_in = outbuf;     /* signal write error */
> > -                return Z_BUF_ERROR;
> > -            }
> > -            return Z_OK;
> > -        }
> > -        code += (unsigned)last << left;     /* middle (or high) bits of code */
> > -        left += 8;
> > -        chunk--;
> > -        if (bits > left) {                  /* need more bits */
> > -            if (NEXT() == -1)               /* can't end in middle of code */
> > -                return Z_BUF_ERROR;
> > -            code += (unsigned)last << left; /* high bits of code */
> > -            left += 8;
> > -            chunk--;
> > -        }
> > -        code &= mask;                       /* mask to current code length */
> > -        left -= bits;                       /* number of unused bits */
> > -        rem = (unsigned)last >> (8 - left); /* unused bits from last byte */
> > -
> > -        /* process clear code (256) */
> > -        if (code == 256 && flags) {
> > -            FLUSHCODE();
> > -            bits = 9;                       /* initialize bits and mask */
> > -            mask = 0x1ff;
> > -            end = 255;                      /* empty table */
> > -            continue;                       /* get next code */
> > -        }
> > -
> > -        /* special code to reuse last match */
> > -        temp = code;                        /* save the current code */
> > -        if (code > end) {
> > -            /* Be picky on the allowed code here, and make sure that the code
> > -               we drop through (prev) will be a valid index so that random
> > -               input does not cause an exception.  The code != end + 1 check is
> > -               empirically derived, and not checked in the original uncompress
> > -               code.  If this ever causes a problem, that check could be safely
> > -               removed.  Leaving this check in greatly improves gun's ability
> > -               to detect random or corrupted input after a compress header.
> > -               In any case, the prev > end check must be retained. */
> > -            if (code != end + 1 || prev > end) {
> > -                strm->msg = (char *)"invalid lzw code";
> > -                return Z_DATA_ERROR;
> > -            }
> > -            match[stack++] = (unsigned char)final;
> > -            code = prev;
> > -        }
> > -
> > -        /* walk through linked list to generate output in reverse order */
> > -        while (code >= 256) {
> > -            match[stack++] = suffix[code];
> > -            code = prefix[code];
> > -        }
> > -        match[stack++] = (unsigned char)code;
> > -        final = code;
> > -
> > -        /* link new table entry */
> > -        if (end < mask) {
> > -            end++;
> > -            prefix[end] = (unsigned short)prev;
> > -            suffix[end] = (unsigned char)final;
> > -        }
> > -
> > -        /* set previous code for next iteration */
> > -        prev = temp;
> > -
> > -        /* write output in forward order */
> > -        while (stack > SIZE - outcnt) {
> > -            while (outcnt < SIZE)
> > -                outbuf[outcnt++] = match[--stack];
> > -            if (out(&outd, outbuf, outcnt)) {
> > -                strm->next_in = outbuf; /* signal write error */
> > -                return Z_BUF_ERROR;
> > -            }
> > -            outcnt = 0;
> > -        }
> > -        do {
> > -            outbuf[outcnt++] = match[--stack];
> > -        } while (stack);
> > -
> > -        /* loop for next code with final and prev as the last match, rem and
> > -           left provide the first 0..7 bits of the next code, end is the last
> > -           valid table entry */
> > -    }
> > -}
> > -
> > -
> > -/* Decompress a gzip file from infile to outfile.  strm is assumed to have been
> > -   successfully initialized with inflateBackInit().  The input file may consist
> > -   of a series of gzip streams, in which case all of them will be decompressed
> > -   to the output file.  If outfile is -1, then the gzip stream(s) integrity is
> > -   checked and nothing is written.
> > -
> > -   The return value is a zlib error code: Z_MEM_ERROR if out of memory,
> > -   Z_DATA_ERROR if the header or the compressed data is invalid, or if the
> > -   trailer CRC-32 check or length doesn't match, Z_BUF_ERROR if the input ends
> > -   prematurely or a write error occurs, or Z_ERRNO if junk (not a another gzip
> > -   stream) follows a valid gzip stream.
> > - */
> > -static int gunpipe(z_stream *strm, int infile, unsigned long *offs, int nbytes, int outfile, uint32_t *checksum, void *dgst)
> > -{
> > -    int ret, first, last;
> > -    unsigned have, flags, len;
> > -    unsigned char *next;
> > -    struct ind ind, *indp;
> > -    struct outd outd;
> > -
> > -    /* setup input buffer */
> > -    ind.infile = infile;
> > -    ind.inbuf = inbuf;
> > -    ind.nbytes = nbytes;
> > -    ind.total = nbytes; /* Used just for progress */
> > -    ind.percent = 0;
> > -    ind.offs = offs;
> > -    ind.checksum = (unsigned long *)checksum;
> > -    ind.dgst = dgst; /* digest for computing hashes */
> > -    indp = &ind;
> > -
> > -    /* decompress concatenated gzip streams */
> > -    have = 0;                               /* no input data read in yet */
> > -    first = 1;                              /* looking for first gzip header */
> > -    strm->next_in = Z_NULL;                 /* so Z_BUF_ERROR means EOF */
> > -    for (;;) {
> > -        /* look for the two magic header bytes for a gzip stream */
> > -        if (NEXT() == -1) {
> > -            ret = Z_OK;
> > -            break;                          /* empty gzip stream is ok */
> > -        }
> > -        if (last != 31 || (NEXT() != 139 && last != 157)) {
> > -            strm->msg = (char *)"incorrect header check";
> > -            ret = first ? Z_DATA_ERROR : Z_ERRNO;
> > -            break;                          /* not a gzip or compress header */
> > -        }
> > -        first = 0;                          /* next non-header is junk */
> > -
> > -        /* process a compress (LZW) file -- can't be concatenated after this */
> > -        if (last == 157) {
> > -            ret = lunpipe(have, next, indp, outfile, strm);
> > -            break;
> > -        }
> > -
> > -        /* process remainder of gzip header */
> > -        ret = Z_BUF_ERROR;
> > -        if (NEXT() != 8) {                  /* only deflate method allowed */
> > -            if (last == -1) break;
> > -            strm->msg = (char *)"unknown compression method";
> > -            ret = Z_DATA_ERROR;
> > -            break;
> > -        }
> > -        flags = NEXT();                     /* header flags */
> > -        NEXT();                             /* discard mod time, xflgs, os */
> > -        NEXT();
> > -        NEXT();
> > -        NEXT();
> > -        NEXT();
> > -        NEXT();
> > -        if (last == -1) break;
> > -        if (flags & 0xe0) {
> > -            strm->msg = (char *)"unknown header flags set";
> > -            ret = Z_DATA_ERROR;
> > -            break;
> > -        }
> > -        if (flags & 4) {                    /* extra field */
> > -            len = NEXT();
> > -            len += (unsigned)(NEXT()) << 8;
> > -            if (last == -1) break;
> > -            while (len > have) {
> > -                len -= have;
> > -                have = 0;
> > -                if (NEXT() == -1) break;
> > -                len--;
> > -            }
> > -            if (last == -1) break;
> > -            have -= len;
> > -            next += len;
> > -        }
> > -        if (flags & 8)                      /* file name */
> > -            while (NEXT() != 0 && last != -1)
> > -                ;
> > -        if (flags & 16)                     /* comment */
> > -            while (NEXT() != 0 && last != -1)
> > -                ;
> > -        if (flags & 2) {                    /* header crc */
> > -            NEXT();
> > -            NEXT();
> > -        }
> > -        if (last == -1) break;
> > -
> > -        /* set up output */
> > -        outd.outfile = outfile;
> > -        outd.check = 1;
> > -        outd.crc = crc32(0L, Z_NULL, 0);
> > -        outd.total = 0;
> > -
> > -        /* decompress data to output */
> > -        strm->next_in = next;
> > -        strm->avail_in = have;
> > -        ret = inflateBack(strm, in, indp, out, &outd);
> > -        if (ret != Z_STREAM_END) break;
> > -        next = strm->next_in;
> > -        have = strm->avail_in;
> > -        strm->next_in = Z_NULL;             /* so Z_BUF_ERROR means EOF */
> > -
> > -        /* check trailer */
> > -        ret = Z_BUF_ERROR;
> > -        if (NEXT() != (int)(outd.crc & 0xff) ||
> > -            NEXT() != (int)((outd.crc >> 8) & 0xff) ||
> > -            NEXT() != (int)((outd.crc >> 16) & 0xff) ||
> > -            NEXT() != (int)((outd.crc >> 24) & 0xff)) {
> > -            /* crc error */
> > -            if (last != -1) {
> > -                strm->msg = (char *)"incorrect data check";
> > -                ret = Z_DATA_ERROR;
> > -            }
> > -            break;
> > -        }
> > -        if (NEXT() != (int)(outd.total & 0xff) ||
> > -            NEXT() != (int)((outd.total >> 8) & 0xff) ||
> > -            NEXT() != (int)((outd.total >> 16) & 0xff) ||
> > -            NEXT() != (int)((outd.total >> 24) & 0xff)) {
> > -            /* length error */
> > -            if (last != -1) {
> > -                strm->msg = (char *)"incorrect length check";
> > -                ret = Z_DATA_ERROR;
> > -            }
> > -            break;
> > -        }
> > -
> > -        /* go back and look for another gzip stream */
> > -    }
> > -
> > -    /* clean up and return */
> > -    return ret;
> > -}
> > -
> > -/* Process the gun command line arguments.  See the command syntax near the
> > -   beginning of this source file. */
> > -int decompress_image(int infile, unsigned long *offs, int nbytes,
> > -     int outfile, uint32_t *checksum, void *dgst)
> > -{
> > -    int ret;
> > -    unsigned char *window;
> > -    z_stream strm;
> > -
> > -    *checksum = 0;
> > -
> > -    /* initialize inflateBack state for repeated use */
> > -    window = match;                         /* reuse LZW match buffer */
> > -    strm.zalloc = Z_NULL;
> > -    strm.zfree = Z_NULL;
> > -    strm.opaque = Z_NULL;
> > -    ret = inflateBackInit(&strm, 15, window);
> > -    if (ret != Z_OK) {
> > -        ERROR("gun out of memory error--aborting\n");
> > -        return 1;
> > -    }
> > -    errno = 0;
> > -    ret = gunpipe(&strm, infile, offs, nbytes, outfile, checksum, dgst);
> > -    /* clean up */
> > -    inflateBackEnd(&strm);
> > -    return ret;
> > -}
> > diff --git a/core/cpio_utils.c b/core/cpio_utils.c
> > index 85f3fc0..26f155c 100644
> > --- a/core/cpio_utils.c
> > +++ b/core/cpio_utils.c
> > @@ -5,10 +5,14 @@
> >   * SPDX-License-Identifier:     GPL-2.0-or-later
> >   */
> >
> > +#include <stdbool.h>
> >  #include <stdlib.h>
> >  #include <stdio.h>
> >  #include <unistd.h>
> >  #include <errno.h>
> > +#ifdef CONFIG_GUNZIP
> > +#include <zlib.h>
> > +#endif
> >
> >  #include "generated/autoconf.h"
> >  #include "cpiohdr.h"
> > @@ -39,7 +43,7 @@ static int get_cpiohdr(unsigned char *buf, unsigned long *size,
> >       return 0;
> >  }
> >
> > -int fill_buffer(int fd, unsigned char *buf, unsigned int nbytes, unsigned long *offs,
> > +static int fill_buffer(int fd, unsigned char *buf, unsigned int nbytes, unsigned long *offs,
> >       uint32_t *checksum, void *dgst)
> >  {
> >       ssize_t len;
> > @@ -99,72 +103,233 @@ static int copy_write(void *out, const void *buf, unsigned int len)
> >       return 0;
> >  }
> >
> > +typedef int (*PipelineStep)(void *state, void *buffer, size_t size);
> > +
> > +struct InputState
> > +{
> > +     int fdin;
> > +     unsigned int nbytes;
> > +     unsigned long *offs;
> > +     void *dgst;     /* use a private context for HASH */
> > +     uint32_t checksum;
> > +};
> > +
> > +static int input_step(void *state, void *buffer, size_t size)
> > +{
> > +     struct InputState *s = (struct InputState *)state;
> > +     if (size >= s->nbytes) {
> > +             size = s->nbytes;
> > +     }
> > +     int ret = fill_buffer(s->fdin, buffer, size, s->offs, &s->checksum, s->dgst);
> > +     if (ret < 0) {
> > +             return ret;
> > +     }
> > +     s->nbytes -= size;
> > +     return ret;
> > +}
> > +
> > +struct DecryptState
> > +{
> > +     PipelineStep upstream_step;
> > +     void *upstream_state;
> > +
> > +     void *dcrypt;   /* use a private context for decryption */
> > +     uint8_t input[BUFF_SIZE];
> > +     uint8_t output[BUFF_SIZE + AES_BLOCK_SIZE];
> > +     int outlen;
> > +     bool eof;
> > +};
>
> ok, you define several stages and you will set input / output of each
> stage later. Got it.

Absolutely. Conceptually, I define a pipeline. Any given step has an
input buffer and an output buffer. Pending output data is simply made
available to the downstream step. If the output buffer is empty, more
input data is processed. If the input buffer is empty, data is pulled
from the upstream step. When no more data can be produced, zero is
returned.
>
>
> > +
> > +static int decrypt_step(void *state, void *buffer, size_t size)
> > +{
> > +     struct DecryptState *s = (struct DecryptState *)state;
> > +     int ret;
> > +     int inlen;
> > +
> > +     if (s->outlen != 0) {
> > +             if (size > s->outlen) {
> > +                     size = s->outlen;
> > +             }
> > +             memcpy(buffer, s->output, size);
> > +             s->outlen -= size;
> > +             memmove(s->output, s->output + size, s->outlen);
> > +             return size;
> > +     }
> > +
> > +     ret = s->upstream_step(s->upstream_state, s->input, sizeof s->input);
> > +     if (ret < 0) {
> > +             return ret;
> > +     }
> > +
> > +     inlen = ret;
> > +
> > +     if (!s->eof) {
> > +             if (inlen != 0) {
> > +                     ret = swupdate_DECRYPT_update(s->dcrypt,
> > +                             s->output, &s->outlen, s->input, inlen);
> > +             } else {
> > +                     /*
> > +                      * Finalise the decryption. Further plaintext bytes may be written at
> > +                      * this stage.
> > +                      */
> > +                     ret = swupdate_DECRYPT_final(s->dcrypt,
> > +                             s->output, &s->outlen);
> > +                     s->eof = true;
> > +             }
> > +             if (ret < 0) {
> > +                     return ret;
> > +             }
> > +     }
> > +
> > +     if (s->outlen != 0) {
> > +             if (size > s->outlen) {
> > +                     size = s->outlen;
> > +             }
> > +             memcpy(buffer, s->output, size);
> > +             s->outlen -= size;
> > +             memmove(s->output, s->output + size, s->outlen);
> > +             return size;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +#ifdef CONFIG_GUNZIP
> > +
> > +struct GunzipState
> > +{
> > +     PipelineStep upstream_step;
> > +     void *upstream_state;
> > +
> > +     z_stream strm;
> > +     bool initialized;
> > +     uint8_t input[BUFF_SIZE];
> > +     bool eof;
> > +};
> > +
> > +static int gunzip_step(void *state, void *buffer, size_t size)
> > +{
> > +     struct GunzipState *s = (struct GunzipState *)state;
> > +     int ret;
> > +     int outlen = 0;
> > +
> > +     s->strm.next_out = buffer;
> > +     s->strm.avail_out = size;
> > +     while (outlen == 0) {
> > +             if (s->strm.avail_in == 0) {
> > +                     ret = s->upstream_step(s->upstream_state, s->input, sizeof s->input);
> > +                     if (ret < 0) {
> > +                             return ret;
> > +                     }
> > +                     s->strm.avail_in = ret;
> > +                     s->strm.next_in = s->input;
> > +             }
> > +             if (s->eof) {
> > +                     break;
> > +             }
> > +
> > +             ret = inflate(&s->strm, Z_NO_FLUSH);
> > +             outlen = size - s->strm.avail_out;
> > +             if (ret == Z_STREAM_END) {
> > +                     s->eof = true;
> > +                     break;
> > +             }
> > +             if (ret != Z_OK && ret != Z_BUF_ERROR) {
> > +                     ERROR("inflate failed (returned %d)", ret);
> > +                     return -1;
> > +             }
> > +     }
> > +     return outlen;
> > +}
> > +
> > +#endif
> > +
> >  int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs, unsigned long long seek,
> >       int skip_file, int __attribute__ ((__unused__)) compressed,
> >       uint32_t *checksum, unsigned char *hash, int encrypted, writeimage callback)
> >  {
> > -     unsigned long size;
> > -     unsigned char *in = NULL, *inbuf;
> > -     unsigned char *decbuf = NULL;
> > -     unsigned long filesize = nbytes;
> >       unsigned int percent, prevpercent = 0;
> >       int ret = 0;
> > -     void *dgst = NULL;      /* use a private context for HASH */
> > -     void *dcrypt = NULL;    /* use a private context for decryption */
> >       int len;
> >       unsigned char md_value[64]; /*
> >                                    *  Maximum hash is 64 bytes for SHA512
> >                                    *  and we use sha256 in swupdate
> >                                    */
> >       unsigned int md_len = 0;
> > -     unsigned char *aes_key;
> > -     unsigned char *ivt;
> > +     unsigned char *aes_key = NULL;
> > +     unsigned char *ivt = NULL;
> >       unsigned char *salt;
> >
> > +     struct InputState input_state = {
> > +             .fdin = fdin,
> > +             .nbytes = nbytes,
> > +             .offs = offs,
> > +             .dgst = NULL,
> > +             .checksum = 0
> > +     };
> > +
> > +     struct DecryptState decrypt_state = {
> > +             .upstream_step = NULL, .upstream_state = NULL,
> > +             .dcrypt = NULL,
> > +             .outlen = 0, .eof = false
> > +     };
> > +
> > +#ifdef CONFIG_GUNZIP
> > +     struct GunzipState gunzip_state = {
> > +             .upstream_step = NULL, .upstream_state = NULL,
> > +             .strm = {
> > +                     .zalloc = Z_NULL, .zfree = Z_NULL, .opaque = Z_NULL,
> > +                     .avail_in = 0, .next_in = Z_NULL,
> > +                     .avail_out = 0, .next_out = Z_NULL
> > +             },
> > +             .initialized = false,
> > +             .eof = false
> > +     };
> > +#endif
> > +
> > +     PipelineStep step = NULL;
> > +     void *state = NULL;
> > +     uint8_t buffer[BUFF_SIZE];
> > +
> >       if (!callback) {
> >               callback = copy_write;
> >       }
> >
> > -     if (IsValidHash(hash)) {
> > -             dgst = swupdate_HASH_init(SHA_DEFAULT);
> > -             if (!dgst)
> > -                     return -EFAULT;
> > -     }
> > -
> >       if (checksum)
> >               *checksum = 0;
> >
> > -     /*
> > -      * Simultaneous compression and decryption of images
> > -      * is not (yet ?) supported
> > -      */
> > -     if (compressed && encrypted) {
> > -             ERROR("encrypted zip images are not yet supported -- aborting\n");
> > -             return -EINVAL;
> > +     if (IsValidHash(hash)) {
> > +             input_state.dgst = swupdate_HASH_init(SHA_DEFAULT);
> > +             if (!input_state.dgst)
> > +                     return -EFAULT;
> >       }
> > -
> > -     in = (unsigned char *)malloc(BUFF_SIZE);
> > -     if (!in)
> > -             return -ENOMEM;
> > -
> > +
> >       if (encrypted) {
> > -             decbuf = (unsigned char *)calloc(1, BUFF_SIZE + AES_BLOCK_SIZE);
> > -             if (!decbuf) {
> > -                     ret = -ENOMEM;
> > -                     goto copyfile_exit;
> > -             }
> > -
> >               aes_key = get_aes_key();
> >               ivt = get_aes_ivt();
> >               salt = get_aes_salt();
> > -             dcrypt = swupdate_DECRYPT_init(aes_key, ivt, salt);
> > -             if (!dcrypt) {
> > +             decrypt_state.dcrypt = swupdate_DECRYPT_init(aes_key, ivt, salt);
> > +             if (!decrypt_state.dcrypt) {
> >                       ERROR("decrypt initialization failure, aborting");
> >                       ret = -EFAULT;
> >                       goto copyfile_exit;
> >               }
> >       }
> >
> > +     if (compressed) {
> > +#ifdef CONFIG_GUNZIP
> > +             if (inflateInit2(&gunzip_state.strm, 16 + MAX_WBITS) != Z_OK) {
>
> This is the core of the whole thing - thanks for the tip, I am not aware
> of it. But it must be somehow documented without checking into Zlib
> manual. Please add comments to indicate that initializing with +16 means
> gunzip header.

I will, better be explicit. I also learned this trick by chance. gun.c
also contained support from LZW decompression that probably noone
needs these days.
>
>
>
> > +                     ERROR("inflateInit2 failed");
> > +                     ret = -EFAULT;
> > +                     goto copyfile_exit;
> > +             }
> > +             gunzip_state.initialized = true;
> > +#else
> > +             TRACE("Request decompressing, but CONFIG_GUNZIP not set !");
> > +             return -1;

Mmh, I cut'n pasted this from decompress_image; rather than "return
-1" it should say "ret = -1; goto copyfile_exit;"
>
> > +#endif
> > +     }
> > +
> >       if (seek) {
> >               int fdout = (out != NULL) ? *(int *)out : -1;
> >               TRACE("offset has been defined: %llu bytes\n", seek);
> > @@ -175,74 +340,66 @@ int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs, unsi
> >               }
> >       }
> >
> > -     int fdout = (out != NULL) ? *(int *)out : -1;
> > +#ifdef CONFIG_GUNZIP
> >       if (compressed) {
> > -             ret = decompress_image(fdin, offs, nbytes, fdout, checksum, dgst);
> > -             if (ret < 0) {
> > -                     ERROR("gunzip failure %d (errno %d) -- aborting\n", ret, errno);
> > -                     goto copyfile_exit;
> > +             if (encrypted) {
> > +                     decrypt_state.upstream_step = &input_step;
> > +                     decrypt_state.upstream_state = &input_state;
> > +                     gunzip_state.upstream_step = &decrypt_step;
> > +                     gunzip_state.upstream_state = &decrypt_state;
> > +             } else {
> > +                     gunzip_state.upstream_step = &input_step;
> > +                     gunzip_state.upstream_state = &input_state;
> >               }
> > +             step = &gunzip_step;
> > +             state = &gunzip_state;
> >       } else {
> > +#endif
> > +             if (encrypted) {
> > +                     decrypt_state.upstream_step = &input_step;
> > +                     decrypt_state.upstream_state = &input_state;
> > +                     step = &decrypt_step;
> > +                     state = &decrypt_state;
> > +             } else {
> > +                     step = &input_step;
> > +                     state = &input_state;
> > +             }
> > +#ifdef CONFIG_GUNZIP
> > +     }
> > +#endif
> >
> > -     while (nbytes > 0) {
> > -             size = (nbytes < BUFF_SIZE ? nbytes : BUFF_SIZE);
> > -
> > -             if ((ret = fill_buffer(fdin, in, size, offs, checksum, dgst) < 0)) {
> > +     for (;;) {
> > +             ret = step(state, buffer, sizeof buffer);
> > +             if (ret < 0) {
> >                       goto copyfile_exit;
> >               }
> > -
> > -             nbytes -= size;
> > -             if (skip_file)
> > +             if (ret == 0) {
> > +                     break;
> > +             }
> > +             if (skip_file) {
> >                       continue;
> > -
> > -             inbuf = in;
> > -             len = size;
> > -
> > -             if (encrypted) {
> > -                     ret = swupdate_DECRYPT_update(dcrypt, decbuf,
> > -                             &len, in, size);
> > -                     if (ret < 0)
> > -                             goto copyfile_exit;
> > -                     inbuf = decbuf;
> >               }
> > -
> > +             len = ret;
> >               /*
> >                * If there is no enough place,
> >                * returns an error and close the output file that
> >                * results corrupted. This lets the cleanup routine
> >                * to remove it
> >                */
> > -             if (callback(out, inbuf, len) < 0) {
> > -                     ret =-ENOSPC;
> > +             if (callback(out, buffer, len) < 0) {
> > +                     ret = -ENOSPC;
> >                       goto copyfile_exit;
> >               }
> >
> > -             percent = (unsigned int)(((double)(filesize - nbytes)) * 100 / filesize);
> > +             percent = (unsigned)(100ULL * (nbytes - input_state.nbytes) / nbytes);
> >               if (percent != prevpercent) {
> >                       prevpercent = percent;
> >                       swupdate_progress_update(percent);
> >               }
> >       }
> >
> > -     }
> > -
> > -     /*
> > -      * Finalise the decryption. Further plaintext bytes may be written at
> > -      * this stage.
> > -      */
> > -     if (encrypted) {
> > -             ret = swupdate_DECRYPT_final(dcrypt, decbuf, &len);
> > -             if (ret < 0)
> > -                     goto copyfile_exit;
> > -             if (callback(out, decbuf, len) < 0) {
> > -                     ret =-ENOSPC;
> > -                     goto copyfile_exit;
> > -             }
> > -     }
> > -
> > -
> >       if (IsValidHash(hash)) {
> > -             if (swupdate_HASH_final(dgst, md_value, &md_len) < 0) {
> > +             if (swupdate_HASH_final(input_state.dgst, md_value, &md_len) < 0) {
> >                       ret = -EFAULT;
> >                       goto copyfile_exit;
> >               }
> > @@ -266,21 +423,26 @@ int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs, unsi
> >               }
> >       }
> >
> > -     fill_buffer(fdin, in, NPAD_BYTES(*offs), offs, checksum, NULL);
> > +     fill_buffer(fdin, buffer, NPAD_BYTES(*offs), offs, checksum, NULL);
> > +
> > +     if (checksum != NULL) {
> > +             *checksum = input_state.checksum;
> > +     }
> >
> >       ret = 0;
> >
> >  copyfile_exit:
> > -     if (in)
> > -             free(in);
> > -     if (dcrypt) {
> > -             swupdate_DECRYPT_cleanup(dcrypt);
> > +     if (decrypt_state.dcrypt) {
> > +             swupdate_DECRYPT_cleanup(decrypt_state.dcrypt);
> > +     }
> > +     if (input_state.dgst) {
> > +             swupdate_HASH_cleanup(input_state.dgst);
> >       }
> > -     if (decbuf)
> > -             free(decbuf);
> > -     if (dgst) {
> > -             swupdate_HASH_cleanup(dgst);
> > +#ifdef CONFIG_GUNZIP
> > +     if (gunzip_state.initialized) {
> > +             inflateEnd(&gunzip_state.strm);
> >       }
> > +#endif
> >
> >       return ret;
> >  }
> > diff --git a/doc/source/roadmap.rst b/doc/source/roadmap.rst
> > index 0502cbe..bc43b9f 100644
> > --- a/doc/source/roadmap.rst
> > +++ b/doc/source/roadmap.rst
> > @@ -108,11 +108,6 @@ Current release supports verified images. That means that a handler
> >  written in Lua could be now be part of the compound image, because
> >  a unauthenticated handler cannot run.
> >
> > -Encryption of compressed artifacts
> > -==================================
> > -
> > -Currently, encrypted artifact are not compressed. Allow to compress artifacts before encryption.
> > -
> >  Support for evaluation boards
> >  =============================
> >
> > diff --git a/handlers/ubivol_handler.c b/handlers/ubivol_handler.c
> > index 25ac837..2c4f95e 100644
> > --- a/handlers/ubivol_handler.c
> > +++ b/handlers/ubivol_handler.c
> > @@ -19,6 +19,7 @@
> >  #include "handler.h"
> >  #include "flash.h"
> >  #include "util.h"
> > +#include "sslapi.h"
> >
> >  void ubi_handler(void);
> >
> > @@ -43,6 +44,9 @@ static int update_volume(libubi_t libubi, struct img_type *img,
> >       char sbuf[128];
> >
> >       bytes = img->size;
> > +     if (img->is_encrypted) {
> > +             bytes -= AES_BLOCK_SIZE;
> > +     }
>
> This is a different issue - it should be fixed in a separate patch.

I found this issue because updating UBI volumes from encrypted images
would fail with the modified code, but I can't remember if it worked
with the original code. I will remove that part if you wish. That
bytes >= AES_BLOCK_SIZE should also be checked, BTW.
>
>
> >
> >       if (!libubi) {
> >               ERROR("Request to write into UBI, but no UBI on system");
> > diff --git a/include/util.h b/include/util.h
> > index d43cd8c..bff9436 100644
> > --- a/include/util.h
> > +++ b/include/util.h
> > @@ -130,8 +130,6 @@ char *sdup(const char *str);
> >   */
> >  typedef int (*writeimage) (void *out, const void *buf, unsigned int len);
> >
> > -int fill_buffer(int fd, unsigned char *buf, unsigned int nbytes, unsigned long *offs,
> > -     uint32_t *checksum, void *dgst);
> >  int openfile(const char *filename);
> >  int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs,
> >       unsigned long long seek,
> > @@ -167,22 +165,6 @@ int get_install_info(sourcetype *source, char *buf, size_t len);
> >
> >  unsigned long long ustrtoull(const char *cp, char **endp, unsigned int base);
> >
> > -#ifdef CONFIG_GUNZIP
> > -int decompress_image(int infile, unsigned long *offs, int nbytes,
> > -     int outfile, uint32_t *checksum, void *dgst);
> > -#else
> > -static inline int decompress_image(int __attribute__ ((__unused__))infile,
> > -                unsigned long __attribute__ ((__unused__)) *offs,
> > -                int __attribute__ ((__unused__)) nbytes,
> > -                int __attribute__ ((__unused__)) outfile,
> > -                uint32_t __attribute__ ((__unused__)) *checksum,
> > -                void __attribute__ ((__unused__)) *dgst) {
> > -
> > -             TRACE("Request decompressing, but CONFIG_GUNZIP not set !");
> > -     return -1;
> > -}
> > -#endif
> > -
> >  const char* get_tmpdir(void);
> >  const char* get_tmpdirscripts(void);
> >
> >
>
> My bigger concern is that the patch is replacing a central piece inside
> SWUpdate. Can you ask which kind of tests you have already done and
> which regressions tests ?

I understand, this is indeed a significant change and I won't take it
bad if you run your own tests before submitting my patch.
I successfully compiled swupdate with and without gzip support.
I tested the modified code on real hardware with the following configurations:
- Digi ccimx6 (eMMC GPT partitions, raw ext4 images):
  - no compression, no encryption
  - compression only
  - encryption only
  - encryption+compression
- Digi ccimx6ul (NAND partitions, UBI volumes, UBIFS images):
  - no compression, no encryption
  - encryption only
In UBIFS images file data is normally already Zlib- or LZO-compressed,
so I have not tested the scenarios that involved gzip compression.
I have not conducted any performance check (execution time, RAM usage).
>
>
> Thanks,
> Stefano Babic
>
> --
> =====================================================================
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
> =====================================================================
>
Stefano Babic Feb. 4, 2018, 6:44 p.m. UTC | #3
Hi Achille,

On 04/02/2018 19:28, Achille Fouilleul wrote:
> Hi Stefano,
> 
> Thanks for considering and reviewing my patch.
> 
> On Sun, Feb 4, 2018 at 12:22 PM, Stefano Babic <sbabic@denx.de> wrote:
>>
>> Hi Achille,
>>
>> On 01/02/2018 13:41, Achille Fouilleul wrote:
>>> Signed-off-by: Achille Fouilleul <achille.fouilleul@gadz.org>
>>> ---
>>>  Makefile                  |   2 +-
>>>  archival/Makefile         |   1 -
>>>  archival/gun.c            | 519 ----------------------------------------------
>>>  core/cpio_utils.c         | 338 ++++++++++++++++++++++--------
>>>  doc/source/roadmap.rst    |   5 -
>>>  handlers/ubivol_handler.c |   4 +
>>>  include/util.h            |  18 --
>>>  7 files changed, 255 insertions(+), 632 deletions(-)
>>>  delete mode 100644 archival/Makefile
>>>  delete mode 100644 archival/gun.c
>>>
>>
>> I appreciate your work cleaning up the code and dropping the old gun.c,
>> this was also one point in my TODO list.
> 
> Yes! Two birds, one stone: support one more option
> (compression+encryption) with (hopefully) cleaner code.
>>
>> Some remarks:
>>
>>
>>> diff --git a/Makefile b/Makefile
>>> index 812a8a4..11fc2f5 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -343,7 +343,7 @@ include $(srctree)/Makefile.flags
>>>  # Defaults to vmlinux, but the arch makefile usually adds further targets
>>>
>>>  objs-y               := core handlers
>>> -libs-y               := archival corelib ipc mongoose parser suricatta bootloader
>>> +libs-y               := corelib ipc mongoose parser suricatta bootloader
>>>  tools-y              := tools
>>>
>>>  swupdate-dirs        := $(objs-y) $(libs-y)
>>> diff --git a/archival/Makefile b/archival/Makefile
>>> deleted file mode 100644
>>> index 5715d0b..0000000
>>> --- a/archival/Makefile
>>> +++ /dev/null
>>> @@ -1 +0,0 @@
>>> -lib-$(CONFIG_GUNZIP)         += gun.o
>>
>> Your match makes CONFIG_GUNZIP useless. HAVE_ZLIB is currently doing the
>> same job. Remove CONFIG_GUNZIP at all and use HAVE_ZLIB when necessary.
> 
> I can see a nuance between CONFIG_GUNZIP and HAVE_ZLIB: one may not
> want to support Zlib compression (CONFIG_GUNZIP=n) even though they
> have the library (HAVE_ZLIB=y). I thought that was the intent, this is
> why I wrote the code that way.

This was the intent in the early beginning, and a reason for the
"archival" directory if more compression archiver will be added. But it
was not extended, and your patch makes it obsolete. It is ok IMHO if you
drop it and the archival directory is removed (drop the Config.in, too).

>>
>>
>>> diff --git a/archival/gun.c b/archival/gun.c
>>> deleted file mode 100644
>>> index ade0379..0000000
>>> --- a/archival/gun.c
>>> +++ /dev/null
>>> @@ -1,519 +0,0 @@
>>> -/* gun.c -- simple gunzip to give an example of the use of inflateBack()
>>> - * Copyright (C) 2003, 2005 Mark Adler
>>> - * For conditions of distribution and use, see copyright notice in zlib.h
>>> -   Version 1.3  12 June 2005  Mark Adler */
>>> -
>>> -/* Version history:
>>> -   1.0  16 Feb 2003  First version for testing of inflateBack()
>>> -   1.1  21 Feb 2005  Decompress concatenated gzip streams
>>> -                     Remove use of "this" variable (C++ keyword)
>>> -                     Fix return value for in()
>>> -                     Improve allocation failure checking
>>> -                     Add typecasting for void * structures
>>> -                     Add -h option for command version and usage
>>> -                     Add a bunch of comments
>>> -   1.2  20 Mar 2005  Add Unix compress (LZW) decompression
>>> -                     Copy file attributes from input file to output file
>>> -   1.3  12 Jun 2005  Add casts for error messages [Oberhumer]
>>> - */
>>> -
>>> -/* external functions and related types and constants */
>>> -#include <stdio.h>          /* fprintf() */
>>> -#include <stdlib.h>         /* malloc(), free() */
>>> -#include <string.h>         /* strerror(), strcmp(), strlen(), memcpy() */
>>> -#include <errno.h>          /* errno */
>>> -#include <fcntl.h>          /* open() */
>>> -#include <unistd.h>         /* read(), write(), close(), chown(), unlink() */
>>> -#include <sys/types.h>
>>> -#include <sys/stat.h>       /* stat(), chmod() */
>>> -#include <utime.h>          /* utime() */
>>> -#include <zlib.h>           /* inflateBackInit(), inflateBack(), */
>>> -                            /* inflateBackEnd(), crc32() */
>>> -#include <util.h>
>>> -#include <progress.h>
>>> -
>>> -/* buffer constants */
>>> -#define SIZE 32768U         /* input and output buffer sizes */
>>> -#define PIECE 16384         /* limits i/o chunks for 16-bit int case */
>>> -
>>> -#define MODULE_NAME "gunzip"
>>> -
>>> -/* structure for infback() to pass to input function in() -- it maintains the
>>> -   input file and a buffer of size SIZE */
>>> -struct ind {
>>> -    int infile;
>>> -    unsigned char *inbuf;
>>> -    unsigned long *offs;
>>> -    unsigned long *checksum;
>>> -    int nbytes;
>>> -    int total;
>>> -    int percent;
>>> -    void *dgst;
>>> -};
>>> -
>>> -/* Load input buffer, assumed to be empty, and return bytes loaded and a
>>> -   pointer to them.  read() is called until the buffer is full, or until it
>>> -   returns end-of-file or error.  Return 0 on error. */
>>> -static unsigned in(void *in_desc, unsigned char **buf)
>>> -{
>>> -    int ret;
>>> -    unsigned len;
>>> -    unsigned char *next;
>>> -    struct ind *me = (struct ind *)in_desc;
>>> -    unsigned int percent;
>>> -
>>> -    next = me->inbuf;
>>> -    *buf = next;
>>> -    len = 0;
>>> -    do {
>>> -        ret = PIECE;
>>> -        if ((unsigned)ret > SIZE - len)
>>> -            ret = (int)(SIZE - len);
>>> -     if (ret > me->nbytes)
>>> -             ret = me->nbytes;
>>> -     ret = fill_buffer(me->infile, next, ret, me->offs, (uint32_t *)me->checksum, me->dgst);
>>> -        if (ret < 0) {
>>> -            len = 0;
>>> -            break;
>>> -        }
>>> -        next += ret;
>>> -        len += ret;
>>> -     me->nbytes -= ret;
>>> -    } while (ret != 0 && len < SIZE);
>>> -    percent = (unsigned int)(((double)(me->total - me->nbytes)) * 100 /
>>> -                 (me->total ? me->total : 1));
>>> -    if (percent != (unsigned int)me->percent) {
>>> -     me->percent = percent;
>>> -     swupdate_progress_update(percent);
>>> -    }
>>> -    return len;
>>> -}
>>> -
>>> -/* structure for infback() to pass to output function out() -- it maintains the
>>> -   output file, a running CRC-32 check on the output and the total number of
>>> -   bytes output, both for checking against the gzip trailer.  (The length in
>>> -   the gzip trailer is stored modulo 2^32, so it's ok if a long is 32 bits and
>>> -   the output is greater than 4 GB.) */
>>> -struct outd {
>>> -    int outfile;
>>> -    int check;                  /* true if checking crc and total */
>>> -    unsigned long crc;
>>> -    unsigned long total;
>>> -};
>>> -
>>> -/* Write output buffer and update the CRC-32 and total bytes written.  write()
>>> -   is called until all of the output is written or an error is encountered.
>>> -   On success out() returns 0.  For a write failure, out() returns 1.  If the
>>> -   output file descriptor is -1, then nothing is written.
>>> - */
>>> -static int out(void *out_desc, unsigned char *buf, unsigned len)
>>> -{
>>> -    int ret;
>>> -    struct outd *me = (struct outd *)out_desc;
>>> -
>>> -    if (me->check) {
>>> -        me->crc = crc32(me->crc, buf, len);
>>> -        me->total += len;
>>> -    }
>>> -    if (me->outfile != -1)
>>> -        do {
>>> -            ret = PIECE;
>>> -            if ((unsigned)ret > len)
>>> -                ret = (int)len;
>>> -            ret = (int)write(me->outfile, buf, ret);
>>> -            if (ret == -1)
>>> -                return 1;
>>> -            buf += ret;
>>> -            len -= ret;
>>> -        } while (len != 0);
>>> -    return 0;
>>> -}
>>> -
>>> -/* next input byte macro for use inside lunpipe() and gunpipe() */
>>> -#define NEXT() (have ? 0 : (have = in(indp, &next)), \
>>> -                last = have ? (have--, (int)(*next++)) : -1)
>>> -
>>> -/* memory for gunpipe() and lunpipe() --
>>> -   the first 256 entries of prefix[] and suffix[] are never used, could
>>> -   have offset the index, but it's faster to waste the memory */
>>> -unsigned char inbuf[SIZE];              /* input buffer */
>>> -unsigned char outbuf[SIZE];             /* output buffer */
>>> -unsigned short prefix[65536];           /* index to LZW prefix string */
>>> -unsigned char suffix[65536];            /* one-character LZW suffix */
>>> -unsigned char match[65280 + 2];         /* buffer for reversed match or gzip
>>> -                                           32K sliding window */
>>> -
>>> -/* throw out what's left in the current bits byte buffer (this is a vestigial
>>> -   aspect of the compressed data format derived from an implementation that
>>> -   made use of a special VAX machine instruction!) */
>>> -#define FLUSHCODE() \
>>> -    do { \
>>> -        left = 0; \
>>> -        rem = 0; \
>>> -        if (chunk > have) { \
>>> -            chunk -= have; \
>>> -            have = 0; \
>>> -            if (NEXT() == -1) \
>>> -                break; \
>>> -            chunk--; \
>>> -            if (chunk > have) { \
>>> -                chunk = have = 0; \
>>> -                break; \
>>> -            } \
>>> -        } \
>>> -        have -= chunk; \
>>> -        next += chunk; \
>>> -        chunk = 0; \
>>> -    } while (0)
>>> -
>>> -/* Decompress a compress (LZW) file from indp to outfile.  The compress magic
>>> -   header (two bytes) has already been read and verified.  There are have bytes
>>> -   of buffered input at next.  strm is used for passing error information back
>>> -   to gunpipe().
>>> -
>>> -   lunpipe() will return Z_OK on success, Z_BUF_ERROR for an unexpected end of
>>> -   file, read error, or write error (a write error indicated by strm->next_in
>>> -   not equal to Z_NULL), or Z_DATA_ERROR for invalid input.
>>> - */
>>> -static int lunpipe(unsigned have, unsigned char *next, struct ind *indp,
>>> -                  int outfile, z_stream *strm)
>>> -{
>>> -    int last;                   /* last byte read by NEXT(), or -1 if EOF */
>>> -    unsigned chunk;             /* bytes left in current chunk */
>>> -    int left;                   /* bits left in rem */
>>> -    unsigned rem;               /* unused bits from input */
>>> -    int bits;                   /* current bits per code */
>>> -    unsigned code;              /* code, table traversal index */
>>> -    unsigned mask;              /* mask for current bits codes */
>>> -    int max;                    /* maximum bits per code for this stream */
>>> -    int flags;                  /* compress flags, then block compress flag */
>>> -    unsigned end;               /* last valid entry in prefix/suffix tables */
>>> -    unsigned temp;              /* current code */
>>> -    unsigned prev;              /* previous code */
>>> -    unsigned final;             /* last character written for previous code */
>>> -    unsigned stack;             /* next position for reversed string */
>>> -    unsigned outcnt;            /* bytes in output buffer */
>>> -    struct outd outd;           /* output structure */
>>> -
>>> -    /* set up output */
>>> -    outd.outfile = outfile;
>>> -    outd.check = 0;
>>> -
>>> -    /* process remainder of compress header -- a flags byte */
>>> -    flags = NEXT();
>>> -    if (last == -1)
>>> -        return Z_BUF_ERROR;
>>> -    if (flags & 0x60) {
>>> -        strm->msg = (char *)"unknown lzw flags set";
>>> -        return Z_DATA_ERROR;
>>> -    }
>>> -    max = flags & 0x1f;
>>> -    if (max < 9 || max > 16) {
>>> -        strm->msg = (char *)"lzw bits out of range";
>>> -        return Z_DATA_ERROR;
>>> -    }
>>> -    if (max == 9)                           /* 9 doesn't really mean 9 */
>>> -        max = 10;
>>> -    flags &= 0x80;                          /* true if block compress */
>>> -
>>> -    /* clear table */
>>> -    bits = 9;
>>> -    mask = 0x1ff;
>>> -    end = flags ? 256 : 255;
>>> -
>>> -    /* set up: get first 9-bit code, which is the first decompressed byte, but
>>> -       don't create a table entry until the next code */
>>> -    if (NEXT() == -1)                       /* no compressed data is ok */
>>> -        return Z_OK;
>>> -    final = prev = (unsigned)last;          /* low 8 bits of code */
>>> -    if (NEXT() == -1)                       /* missing a bit */
>>> -        return Z_BUF_ERROR;
>>> -    if (last & 1) {                         /* code must be < 256 */
>>> -        strm->msg = (char *)"invalid lzw code";
>>> -        return Z_DATA_ERROR;
>>> -    }
>>> -    rem = (unsigned)last >> 1;              /* remaining 7 bits */
>>> -    left = 7;
>>> -    chunk = bits - 2;                       /* 7 bytes left in this chunk */
>>> -    outbuf[0] = (unsigned char)final;       /* write first decompressed byte */
>>> -    outcnt = 1;
>>> -
>>> -    /* decode codes */
>>> -    stack = 0;
>>> -    for (;;) {
>>> -        /* if the table will be full after this, increment the code size */
>>> -        if (end >= mask && bits < max) {
>>> -            FLUSHCODE();
>>> -            bits++;
>>> -            mask <<= 1;
>>> -            mask++;
>>> -        }
>>> -
>>> -        /* get a code of length bits */
>>> -        if (chunk == 0)                     /* decrement chunk modulo bits */
>>> -            chunk = bits;
>>> -        code = rem;                         /* low bits of code */
>>> -        if (NEXT() == -1) {                 /* EOF is end of compressed data */
>>> -            /* write remaining buffered output */
>>> -            if (outcnt && out(&outd, outbuf, outcnt)) {
>>> -                strm->next_in = outbuf;     /* signal write error */
>>> -                return Z_BUF_ERROR;
>>> -            }
>>> -            return Z_OK;
>>> -        }
>>> -        code += (unsigned)last << left;     /* middle (or high) bits of code */
>>> -        left += 8;
>>> -        chunk--;
>>> -        if (bits > left) {                  /* need more bits */
>>> -            if (NEXT() == -1)               /* can't end in middle of code */
>>> -                return Z_BUF_ERROR;
>>> -            code += (unsigned)last << left; /* high bits of code */
>>> -            left += 8;
>>> -            chunk--;
>>> -        }
>>> -        code &= mask;                       /* mask to current code length */
>>> -        left -= bits;                       /* number of unused bits */
>>> -        rem = (unsigned)last >> (8 - left); /* unused bits from last byte */
>>> -
>>> -        /* process clear code (256) */
>>> -        if (code == 256 && flags) {
>>> -            FLUSHCODE();
>>> -            bits = 9;                       /* initialize bits and mask */
>>> -            mask = 0x1ff;
>>> -            end = 255;                      /* empty table */
>>> -            continue;                       /* get next code */
>>> -        }
>>> -
>>> -        /* special code to reuse last match */
>>> -        temp = code;                        /* save the current code */
>>> -        if (code > end) {
>>> -            /* Be picky on the allowed code here, and make sure that the code
>>> -               we drop through (prev) will be a valid index so that random
>>> -               input does not cause an exception.  The code != end + 1 check is
>>> -               empirically derived, and not checked in the original uncompress
>>> -               code.  If this ever causes a problem, that check could be safely
>>> -               removed.  Leaving this check in greatly improves gun's ability
>>> -               to detect random or corrupted input after a compress header.
>>> -               In any case, the prev > end check must be retained. */
>>> -            if (code != end + 1 || prev > end) {
>>> -                strm->msg = (char *)"invalid lzw code";
>>> -                return Z_DATA_ERROR;
>>> -            }
>>> -            match[stack++] = (unsigned char)final;
>>> -            code = prev;
>>> -        }
>>> -
>>> -        /* walk through linked list to generate output in reverse order */
>>> -        while (code >= 256) {
>>> -            match[stack++] = suffix[code];
>>> -            code = prefix[code];
>>> -        }
>>> -        match[stack++] = (unsigned char)code;
>>> -        final = code;
>>> -
>>> -        /* link new table entry */
>>> -        if (end < mask) {
>>> -            end++;
>>> -            prefix[end] = (unsigned short)prev;
>>> -            suffix[end] = (unsigned char)final;
>>> -        }
>>> -
>>> -        /* set previous code for next iteration */
>>> -        prev = temp;
>>> -
>>> -        /* write output in forward order */
>>> -        while (stack > SIZE - outcnt) {
>>> -            while (outcnt < SIZE)
>>> -                outbuf[outcnt++] = match[--stack];
>>> -            if (out(&outd, outbuf, outcnt)) {
>>> -                strm->next_in = outbuf; /* signal write error */
>>> -                return Z_BUF_ERROR;
>>> -            }
>>> -            outcnt = 0;
>>> -        }
>>> -        do {
>>> -            outbuf[outcnt++] = match[--stack];
>>> -        } while (stack);
>>> -
>>> -        /* loop for next code with final and prev as the last match, rem and
>>> -           left provide the first 0..7 bits of the next code, end is the last
>>> -           valid table entry */
>>> -    }
>>> -}
>>> -
>>> -
>>> -/* Decompress a gzip file from infile to outfile.  strm is assumed to have been
>>> -   successfully initialized with inflateBackInit().  The input file may consist
>>> -   of a series of gzip streams, in which case all of them will be decompressed
>>> -   to the output file.  If outfile is -1, then the gzip stream(s) integrity is
>>> -   checked and nothing is written.
>>> -
>>> -   The return value is a zlib error code: Z_MEM_ERROR if out of memory,
>>> -   Z_DATA_ERROR if the header or the compressed data is invalid, or if the
>>> -   trailer CRC-32 check or length doesn't match, Z_BUF_ERROR if the input ends
>>> -   prematurely or a write error occurs, or Z_ERRNO if junk (not a another gzip
>>> -   stream) follows a valid gzip stream.
>>> - */
>>> -static int gunpipe(z_stream *strm, int infile, unsigned long *offs, int nbytes, int outfile, uint32_t *checksum, void *dgst)
>>> -{
>>> -    int ret, first, last;
>>> -    unsigned have, flags, len;
>>> -    unsigned char *next;
>>> -    struct ind ind, *indp;
>>> -    struct outd outd;
>>> -
>>> -    /* setup input buffer */
>>> -    ind.infile = infile;
>>> -    ind.inbuf = inbuf;
>>> -    ind.nbytes = nbytes;
>>> -    ind.total = nbytes; /* Used just for progress */
>>> -    ind.percent = 0;
>>> -    ind.offs = offs;
>>> -    ind.checksum = (unsigned long *)checksum;
>>> -    ind.dgst = dgst; /* digest for computing hashes */
>>> -    indp = &ind;
>>> -
>>> -    /* decompress concatenated gzip streams */
>>> -    have = 0;                               /* no input data read in yet */
>>> -    first = 1;                              /* looking for first gzip header */
>>> -    strm->next_in = Z_NULL;                 /* so Z_BUF_ERROR means EOF */
>>> -    for (;;) {
>>> -        /* look for the two magic header bytes for a gzip stream */
>>> -        if (NEXT() == -1) {
>>> -            ret = Z_OK;
>>> -            break;                          /* empty gzip stream is ok */
>>> -        }
>>> -        if (last != 31 || (NEXT() != 139 && last != 157)) {
>>> -            strm->msg = (char *)"incorrect header check";
>>> -            ret = first ? Z_DATA_ERROR : Z_ERRNO;
>>> -            break;                          /* not a gzip or compress header */
>>> -        }
>>> -        first = 0;                          /* next non-header is junk */
>>> -
>>> -        /* process a compress (LZW) file -- can't be concatenated after this */
>>> -        if (last == 157) {
>>> -            ret = lunpipe(have, next, indp, outfile, strm);
>>> -            break;
>>> -        }
>>> -
>>> -        /* process remainder of gzip header */
>>> -        ret = Z_BUF_ERROR;
>>> -        if (NEXT() != 8) {                  /* only deflate method allowed */
>>> -            if (last == -1) break;
>>> -            strm->msg = (char *)"unknown compression method";
>>> -            ret = Z_DATA_ERROR;
>>> -            break;
>>> -        }
>>> -        flags = NEXT();                     /* header flags */
>>> -        NEXT();                             /* discard mod time, xflgs, os */
>>> -        NEXT();
>>> -        NEXT();
>>> -        NEXT();
>>> -        NEXT();
>>> -        NEXT();
>>> -        if (last == -1) break;
>>> -        if (flags & 0xe0) {
>>> -            strm->msg = (char *)"unknown header flags set";
>>> -            ret = Z_DATA_ERROR;
>>> -            break;
>>> -        }
>>> -        if (flags & 4) {                    /* extra field */
>>> -            len = NEXT();
>>> -            len += (unsigned)(NEXT()) << 8;
>>> -            if (last == -1) break;
>>> -            while (len > have) {
>>> -                len -= have;
>>> -                have = 0;
>>> -                if (NEXT() == -1) break;
>>> -                len--;
>>> -            }
>>> -            if (last == -1) break;
>>> -            have -= len;
>>> -            next += len;
>>> -        }
>>> -        if (flags & 8)                      /* file name */
>>> -            while (NEXT() != 0 && last != -1)
>>> -                ;
>>> -        if (flags & 16)                     /* comment */
>>> -            while (NEXT() != 0 && last != -1)
>>> -                ;
>>> -        if (flags & 2) {                    /* header crc */
>>> -            NEXT();
>>> -            NEXT();
>>> -        }
>>> -        if (last == -1) break;
>>> -
>>> -        /* set up output */
>>> -        outd.outfile = outfile;
>>> -        outd.check = 1;
>>> -        outd.crc = crc32(0L, Z_NULL, 0);
>>> -        outd.total = 0;
>>> -
>>> -        /* decompress data to output */
>>> -        strm->next_in = next;
>>> -        strm->avail_in = have;
>>> -        ret = inflateBack(strm, in, indp, out, &outd);
>>> -        if (ret != Z_STREAM_END) break;
>>> -        next = strm->next_in;
>>> -        have = strm->avail_in;
>>> -        strm->next_in = Z_NULL;             /* so Z_BUF_ERROR means EOF */
>>> -
>>> -        /* check trailer */
>>> -        ret = Z_BUF_ERROR;
>>> -        if (NEXT() != (int)(outd.crc & 0xff) ||
>>> -            NEXT() != (int)((outd.crc >> 8) & 0xff) ||
>>> -            NEXT() != (int)((outd.crc >> 16) & 0xff) ||
>>> -            NEXT() != (int)((outd.crc >> 24) & 0xff)) {
>>> -            /* crc error */
>>> -            if (last != -1) {
>>> -                strm->msg = (char *)"incorrect data check";
>>> -                ret = Z_DATA_ERROR;
>>> -            }
>>> -            break;
>>> -        }
>>> -        if (NEXT() != (int)(outd.total & 0xff) ||
>>> -            NEXT() != (int)((outd.total >> 8) & 0xff) ||
>>> -            NEXT() != (int)((outd.total >> 16) & 0xff) ||
>>> -            NEXT() != (int)((outd.total >> 24) & 0xff)) {
>>> -            /* length error */
>>> -            if (last != -1) {
>>> -                strm->msg = (char *)"incorrect length check";
>>> -                ret = Z_DATA_ERROR;
>>> -            }
>>> -            break;
>>> -        }
>>> -
>>> -        /* go back and look for another gzip stream */
>>> -    }
>>> -
>>> -    /* clean up and return */
>>> -    return ret;
>>> -}
>>> -
>>> -/* Process the gun command line arguments.  See the command syntax near the
>>> -   beginning of this source file. */
>>> -int decompress_image(int infile, unsigned long *offs, int nbytes,
>>> -     int outfile, uint32_t *checksum, void *dgst)
>>> -{
>>> -    int ret;
>>> -    unsigned char *window;
>>> -    z_stream strm;
>>> -
>>> -    *checksum = 0;
>>> -
>>> -    /* initialize inflateBack state for repeated use */
>>> -    window = match;                         /* reuse LZW match buffer */
>>> -    strm.zalloc = Z_NULL;
>>> -    strm.zfree = Z_NULL;
>>> -    strm.opaque = Z_NULL;
>>> -    ret = inflateBackInit(&strm, 15, window);
>>> -    if (ret != Z_OK) {
>>> -        ERROR("gun out of memory error--aborting\n");
>>> -        return 1;
>>> -    }
>>> -    errno = 0;
>>> -    ret = gunpipe(&strm, infile, offs, nbytes, outfile, checksum, dgst);
>>> -    /* clean up */
>>> -    inflateBackEnd(&strm);
>>> -    return ret;
>>> -}
>>> diff --git a/core/cpio_utils.c b/core/cpio_utils.c
>>> index 85f3fc0..26f155c 100644
>>> --- a/core/cpio_utils.c
>>> +++ b/core/cpio_utils.c
>>> @@ -5,10 +5,14 @@
>>>   * SPDX-License-Identifier:     GPL-2.0-or-later
>>>   */
>>>
>>> +#include <stdbool.h>
>>>  #include <stdlib.h>
>>>  #include <stdio.h>
>>>  #include <unistd.h>
>>>  #include <errno.h>
>>> +#ifdef CONFIG_GUNZIP
>>> +#include <zlib.h>
>>> +#endif
>>>
>>>  #include "generated/autoconf.h"
>>>  #include "cpiohdr.h"
>>> @@ -39,7 +43,7 @@ static int get_cpiohdr(unsigned char *buf, unsigned long *size,
>>>       return 0;
>>>  }
>>>
>>> -int fill_buffer(int fd, unsigned char *buf, unsigned int nbytes, unsigned long *offs,
>>> +static int fill_buffer(int fd, unsigned char *buf, unsigned int nbytes, unsigned long *offs,
>>>       uint32_t *checksum, void *dgst)
>>>  {
>>>       ssize_t len;
>>> @@ -99,72 +103,233 @@ static int copy_write(void *out, const void *buf, unsigned int len)
>>>       return 0;
>>>  }
>>>
>>> +typedef int (*PipelineStep)(void *state, void *buffer, size_t size);
>>> +
>>> +struct InputState
>>> +{
>>> +     int fdin;
>>> +     unsigned int nbytes;
>>> +     unsigned long *offs;
>>> +     void *dgst;     /* use a private context for HASH */
>>> +     uint32_t checksum;
>>> +};
>>> +
>>> +static int input_step(void *state, void *buffer, size_t size)
>>> +{
>>> +     struct InputState *s = (struct InputState *)state;
>>> +     if (size >= s->nbytes) {
>>> +             size = s->nbytes;
>>> +     }
>>> +     int ret = fill_buffer(s->fdin, buffer, size, s->offs, &s->checksum, s->dgst);
>>> +     if (ret < 0) {
>>> +             return ret;
>>> +     }
>>> +     s->nbytes -= size;
>>> +     return ret;
>>> +}
>>> +
>>> +struct DecryptState
>>> +{
>>> +     PipelineStep upstream_step;
>>> +     void *upstream_state;
>>> +
>>> +     void *dcrypt;   /* use a private context for decryption */
>>> +     uint8_t input[BUFF_SIZE];
>>> +     uint8_t output[BUFF_SIZE + AES_BLOCK_SIZE];
>>> +     int outlen;
>>> +     bool eof;
>>> +};
>>
>> ok, you define several stages and you will set input / output of each
>> stage later. Got it.
> 
> Absolutely. Conceptually, I define a pipeline. Any given step has an
> input buffer and an output buffer. Pending output data is simply made
> available to the downstream step. If the output buffer is empty, more
> input data is processed. If the input buffer is empty, data is pulled
> from the upstream step. When no more data can be produced, zero is
> returned.

ok

>>
>>
>>> +
>>> +static int decrypt_step(void *state, void *buffer, size_t size)
>>> +{
>>> +     struct DecryptState *s = (struct DecryptState *)state;
>>> +     int ret;
>>> +     int inlen;
>>> +
>>> +     if (s->outlen != 0) {
>>> +             if (size > s->outlen) {
>>> +                     size = s->outlen;
>>> +             }
>>> +             memcpy(buffer, s->output, size);
>>> +             s->outlen -= size;
>>> +             memmove(s->output, s->output + size, s->outlen);
>>> +             return size;
>>> +     }
>>> +
>>> +     ret = s->upstream_step(s->upstream_state, s->input, sizeof s->input);
>>> +     if (ret < 0) {
>>> +             return ret;
>>> +     }
>>> +
>>> +     inlen = ret;
>>> +
>>> +     if (!s->eof) {
>>> +             if (inlen != 0) {
>>> +                     ret = swupdate_DECRYPT_update(s->dcrypt,
>>> +                             s->output, &s->outlen, s->input, inlen);
>>> +             } else {
>>> +                     /*
>>> +                      * Finalise the decryption. Further plaintext bytes may be written at
>>> +                      * this stage.
>>> +                      */
>>> +                     ret = swupdate_DECRYPT_final(s->dcrypt,
>>> +                             s->output, &s->outlen);
>>> +                     s->eof = true;
>>> +             }
>>> +             if (ret < 0) {
>>> +                     return ret;
>>> +             }
>>> +     }
>>> +
>>> +     if (s->outlen != 0) {
>>> +             if (size > s->outlen) {
>>> +                     size = s->outlen;
>>> +             }
>>> +             memcpy(buffer, s->output, size);
>>> +             s->outlen -= size;
>>> +             memmove(s->output, s->output + size, s->outlen);
>>> +             return size;
>>> +     }
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +#ifdef CONFIG_GUNZIP
>>> +
>>> +struct GunzipState
>>> +{
>>> +     PipelineStep upstream_step;
>>> +     void *upstream_state;
>>> +
>>> +     z_stream strm;
>>> +     bool initialized;
>>> +     uint8_t input[BUFF_SIZE];
>>> +     bool eof;
>>> +};
>>> +
>>> +static int gunzip_step(void *state, void *buffer, size_t size)
>>> +{
>>> +     struct GunzipState *s = (struct GunzipState *)state;
>>> +     int ret;
>>> +     int outlen = 0;
>>> +
>>> +     s->strm.next_out = buffer;
>>> +     s->strm.avail_out = size;
>>> +     while (outlen == 0) {
>>> +             if (s->strm.avail_in == 0) {
>>> +                     ret = s->upstream_step(s->upstream_state, s->input, sizeof s->input);
>>> +                     if (ret < 0) {
>>> +                             return ret;
>>> +                     }
>>> +                     s->strm.avail_in = ret;
>>> +                     s->strm.next_in = s->input;
>>> +             }
>>> +             if (s->eof) {
>>> +                     break;
>>> +             }
>>> +
>>> +             ret = inflate(&s->strm, Z_NO_FLUSH);
>>> +             outlen = size - s->strm.avail_out;
>>> +             if (ret == Z_STREAM_END) {
>>> +                     s->eof = true;
>>> +                     break;
>>> +             }
>>> +             if (ret != Z_OK && ret != Z_BUF_ERROR) {
>>> +                     ERROR("inflate failed (returned %d)", ret);
>>> +                     return -1;
>>> +             }
>>> +     }
>>> +     return outlen;
>>> +}
>>> +
>>> +#endif
>>> +
>>>  int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs, unsigned long long seek,
>>>       int skip_file, int __attribute__ ((__unused__)) compressed,
>>>       uint32_t *checksum, unsigned char *hash, int encrypted, writeimage callback)
>>>  {
>>> -     unsigned long size;
>>> -     unsigned char *in = NULL, *inbuf;
>>> -     unsigned char *decbuf = NULL;
>>> -     unsigned long filesize = nbytes;
>>>       unsigned int percent, prevpercent = 0;
>>>       int ret = 0;
>>> -     void *dgst = NULL;      /* use a private context for HASH */
>>> -     void *dcrypt = NULL;    /* use a private context for decryption */
>>>       int len;
>>>       unsigned char md_value[64]; /*
>>>                                    *  Maximum hash is 64 bytes for SHA512
>>>                                    *  and we use sha256 in swupdate
>>>                                    */
>>>       unsigned int md_len = 0;
>>> -     unsigned char *aes_key;
>>> -     unsigned char *ivt;
>>> +     unsigned char *aes_key = NULL;
>>> +     unsigned char *ivt = NULL;
>>>       unsigned char *salt;
>>>
>>> +     struct InputState input_state = {
>>> +             .fdin = fdin,
>>> +             .nbytes = nbytes,
>>> +             .offs = offs,
>>> +             .dgst = NULL,
>>> +             .checksum = 0
>>> +     };
>>> +
>>> +     struct DecryptState decrypt_state = {
>>> +             .upstream_step = NULL, .upstream_state = NULL,
>>> +             .dcrypt = NULL,
>>> +             .outlen = 0, .eof = false
>>> +     };
>>> +
>>> +#ifdef CONFIG_GUNZIP
>>> +     struct GunzipState gunzip_state = {
>>> +             .upstream_step = NULL, .upstream_state = NULL,
>>> +             .strm = {
>>> +                     .zalloc = Z_NULL, .zfree = Z_NULL, .opaque = Z_NULL,
>>> +                     .avail_in = 0, .next_in = Z_NULL,
>>> +                     .avail_out = 0, .next_out = Z_NULL
>>> +             },
>>> +             .initialized = false,
>>> +             .eof = false
>>> +     };
>>> +#endif
>>> +
>>> +     PipelineStep step = NULL;
>>> +     void *state = NULL;
>>> +     uint8_t buffer[BUFF_SIZE];
>>> +
>>>       if (!callback) {
>>>               callback = copy_write;
>>>       }
>>>
>>> -     if (IsValidHash(hash)) {
>>> -             dgst = swupdate_HASH_init(SHA_DEFAULT);
>>> -             if (!dgst)
>>> -                     return -EFAULT;
>>> -     }
>>> -
>>>       if (checksum)
>>>               *checksum = 0;
>>>
>>> -     /*
>>> -      * Simultaneous compression and decryption of images
>>> -      * is not (yet ?) supported
>>> -      */
>>> -     if (compressed && encrypted) {
>>> -             ERROR("encrypted zip images are not yet supported -- aborting\n");
>>> -             return -EINVAL;
>>> +     if (IsValidHash(hash)) {
>>> +             input_state.dgst = swupdate_HASH_init(SHA_DEFAULT);
>>> +             if (!input_state.dgst)
>>> +                     return -EFAULT;
>>>       }
>>> -
>>> -     in = (unsigned char *)malloc(BUFF_SIZE);
>>> -     if (!in)
>>> -             return -ENOMEM;
>>> -
>>> +
>>>       if (encrypted) {
>>> -             decbuf = (unsigned char *)calloc(1, BUFF_SIZE + AES_BLOCK_SIZE);
>>> -             if (!decbuf) {
>>> -                     ret = -ENOMEM;
>>> -                     goto copyfile_exit;
>>> -             }
>>> -
>>>               aes_key = get_aes_key();
>>>               ivt = get_aes_ivt();
>>>               salt = get_aes_salt();
>>> -             dcrypt = swupdate_DECRYPT_init(aes_key, ivt, salt);
>>> -             if (!dcrypt) {
>>> +             decrypt_state.dcrypt = swupdate_DECRYPT_init(aes_key, ivt, salt);
>>> +             if (!decrypt_state.dcrypt) {
>>>                       ERROR("decrypt initialization failure, aborting");
>>>                       ret = -EFAULT;
>>>                       goto copyfile_exit;
>>>               }
>>>       }
>>>
>>> +     if (compressed) {
>>> +#ifdef CONFIG_GUNZIP
>>> +             if (inflateInit2(&gunzip_state.strm, 16 + MAX_WBITS) != Z_OK) {
>>
>> This is the core of the whole thing - thanks for the tip, I am not aware
>> of it. But it must be somehow documented without checking into Zlib
>> manual. Please add comments to indicate that initializing with +16 means
>> gunzip header.
> 
> I will, better be explicit.

ok, thanks.

> I also learned this trick by chance.
> gun.c
> also contained support from LZW decompression that probably noone
> needs these days.

Not needed in SWUpdate, yes.

>>
>>
>>
>>> +                     ERROR("inflateInit2 failed");
>>> +                     ret = -EFAULT;
>>> +                     goto copyfile_exit;
>>> +             }
>>> +             gunzip_state.initialized = true;
>>> +#else
>>> +             TRACE("Request decompressing, but CONFIG_GUNZIP not set !");
>>> +             return -1;
> 
> Mmh, I cut'n pasted this from decompress_image; rather than "return
> -1" it should say "ret = -1; goto copyfile_exit;"

Maybe ret = -EINVAL

>>
>>> +#endif
>>> +     }
>>> +
>>>       if (seek) {
>>>               int fdout = (out != NULL) ? *(int *)out : -1;
>>>               TRACE("offset has been defined: %llu bytes\n", seek);
>>> @@ -175,74 +340,66 @@ int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs, unsi
>>>               }
>>>       }
>>>
>>> -     int fdout = (out != NULL) ? *(int *)out : -1;
>>> +#ifdef CONFIG_GUNZIP
>>>       if (compressed) {
>>> -             ret = decompress_image(fdin, offs, nbytes, fdout, checksum, dgst);
>>> -             if (ret < 0) {
>>> -                     ERROR("gunzip failure %d (errno %d) -- aborting\n", ret, errno);
>>> -                     goto copyfile_exit;
>>> +             if (encrypted) {
>>> +                     decrypt_state.upstream_step = &input_step;
>>> +                     decrypt_state.upstream_state = &input_state;
>>> +                     gunzip_state.upstream_step = &decrypt_step;
>>> +                     gunzip_state.upstream_state = &decrypt_state;
>>> +             } else {
>>> +                     gunzip_state.upstream_step = &input_step;
>>> +                     gunzip_state.upstream_state = &input_state;
>>>               }
>>> +             step = &gunzip_step;
>>> +             state = &gunzip_state;
>>>       } else {
>>> +#endif
>>> +             if (encrypted) {
>>> +                     decrypt_state.upstream_step = &input_step;
>>> +                     decrypt_state.upstream_state = &input_state;
>>> +                     step = &decrypt_step;
>>> +                     state = &decrypt_state;
>>> +             } else {
>>> +                     step = &input_step;
>>> +                     state = &input_state;
>>> +             }
>>> +#ifdef CONFIG_GUNZIP
>>> +     }
>>> +#endif
>>>
>>> -     while (nbytes > 0) {
>>> -             size = (nbytes < BUFF_SIZE ? nbytes : BUFF_SIZE);
>>> -
>>> -             if ((ret = fill_buffer(fdin, in, size, offs, checksum, dgst) < 0)) {
>>> +     for (;;) {
>>> +             ret = step(state, buffer, sizeof buffer);
>>> +             if (ret < 0) {
>>>                       goto copyfile_exit;
>>>               }
>>> -
>>> -             nbytes -= size;
>>> -             if (skip_file)
>>> +             if (ret == 0) {
>>> +                     break;
>>> +             }
>>> +             if (skip_file) {
>>>                       continue;
>>> -
>>> -             inbuf = in;
>>> -             len = size;
>>> -
>>> -             if (encrypted) {
>>> -                     ret = swupdate_DECRYPT_update(dcrypt, decbuf,
>>> -                             &len, in, size);
>>> -                     if (ret < 0)
>>> -                             goto copyfile_exit;
>>> -                     inbuf = decbuf;
>>>               }
>>> -
>>> +             len = ret;
>>>               /*
>>>                * If there is no enough place,
>>>                * returns an error and close the output file that
>>>                * results corrupted. This lets the cleanup routine
>>>                * to remove it
>>>                */
>>> -             if (callback(out, inbuf, len) < 0) {
>>> -                     ret =-ENOSPC;
>>> +             if (callback(out, buffer, len) < 0) {
>>> +                     ret = -ENOSPC;
>>>                       goto copyfile_exit;
>>>               }
>>>
>>> -             percent = (unsigned int)(((double)(filesize - nbytes)) * 100 / filesize);
>>> +             percent = (unsigned)(100ULL * (nbytes - input_state.nbytes) / nbytes);
>>>               if (percent != prevpercent) {
>>>                       prevpercent = percent;
>>>                       swupdate_progress_update(percent);
>>>               }
>>>       }
>>>
>>> -     }
>>> -
>>> -     /*
>>> -      * Finalise the decryption. Further plaintext bytes may be written at
>>> -      * this stage.
>>> -      */
>>> -     if (encrypted) {
>>> -             ret = swupdate_DECRYPT_final(dcrypt, decbuf, &len);
>>> -             if (ret < 0)
>>> -                     goto copyfile_exit;
>>> -             if (callback(out, decbuf, len) < 0) {
>>> -                     ret =-ENOSPC;
>>> -                     goto copyfile_exit;
>>> -             }
>>> -     }
>>> -
>>> -
>>>       if (IsValidHash(hash)) {
>>> -             if (swupdate_HASH_final(dgst, md_value, &md_len) < 0) {
>>> +             if (swupdate_HASH_final(input_state.dgst, md_value, &md_len) < 0) {
>>>                       ret = -EFAULT;
>>>                       goto copyfile_exit;
>>>               }
>>> @@ -266,21 +423,26 @@ int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs, unsi
>>>               }
>>>       }
>>>
>>> -     fill_buffer(fdin, in, NPAD_BYTES(*offs), offs, checksum, NULL);
>>> +     fill_buffer(fdin, buffer, NPAD_BYTES(*offs), offs, checksum, NULL);
>>> +
>>> +     if (checksum != NULL) {
>>> +             *checksum = input_state.checksum;
>>> +     }
>>>
>>>       ret = 0;
>>>
>>>  copyfile_exit:
>>> -     if (in)
>>> -             free(in);
>>> -     if (dcrypt) {
>>> -             swupdate_DECRYPT_cleanup(dcrypt);
>>> +     if (decrypt_state.dcrypt) {
>>> +             swupdate_DECRYPT_cleanup(decrypt_state.dcrypt);
>>> +     }
>>> +     if (input_state.dgst) {
>>> +             swupdate_HASH_cleanup(input_state.dgst);
>>>       }
>>> -     if (decbuf)
>>> -             free(decbuf);
>>> -     if (dgst) {
>>> -             swupdate_HASH_cleanup(dgst);
>>> +#ifdef CONFIG_GUNZIP
>>> +     if (gunzip_state.initialized) {
>>> +             inflateEnd(&gunzip_state.strm);
>>>       }
>>> +#endif
>>>
>>>       return ret;
>>>  }
>>> diff --git a/doc/source/roadmap.rst b/doc/source/roadmap.rst
>>> index 0502cbe..bc43b9f 100644
>>> --- a/doc/source/roadmap.rst
>>> +++ b/doc/source/roadmap.rst
>>> @@ -108,11 +108,6 @@ Current release supports verified images. That means that a handler
>>>  written in Lua could be now be part of the compound image, because
>>>  a unauthenticated handler cannot run.
>>>
>>> -Encryption of compressed artifacts
>>> -==================================
>>> -
>>> -Currently, encrypted artifact are not compressed. Allow to compress artifacts before encryption.
>>> -
>>>  Support for evaluation boards
>>>  =============================
>>>
>>> diff --git a/handlers/ubivol_handler.c b/handlers/ubivol_handler.c
>>> index 25ac837..2c4f95e 100644
>>> --- a/handlers/ubivol_handler.c
>>> +++ b/handlers/ubivol_handler.c
>>> @@ -19,6 +19,7 @@
>>>  #include "handler.h"
>>>  #include "flash.h"
>>>  #include "util.h"
>>> +#include "sslapi.h"
>>>
>>>  void ubi_handler(void);
>>>
>>> @@ -43,6 +44,9 @@ static int update_volume(libubi_t libubi, struct img_type *img,
>>>       char sbuf[128];
>>>
>>>       bytes = img->size;
>>> +     if (img->is_encrypted) {
>>> +             bytes -= AES_BLOCK_SIZE;
>>> +     }
>>
>> This is a different issue - it should be fixed in a separate patch.
> 
> I found this issue because updating UBI volumes from encrypted images
> would fail with the modified code, but I can't remember if it worked
> with the original code.

It does not work, but it is a different issue.

> I will remove that part if you wish. 

No, you should send a second patch for this. This fixes the UBI handler
and have less with adding encryption / compression at once.

> That
> bytes >= AES_BLOCK_SIZE should also be checked, BTW.
>>
>>
>>>
>>>       if (!libubi) {
>>>               ERROR("Request to write into UBI, but no UBI on system");
>>> diff --git a/include/util.h b/include/util.h
>>> index d43cd8c..bff9436 100644
>>> --- a/include/util.h
>>> +++ b/include/util.h
>>> @@ -130,8 +130,6 @@ char *sdup(const char *str);
>>>   */
>>>  typedef int (*writeimage) (void *out, const void *buf, unsigned int len);
>>>
>>> -int fill_buffer(int fd, unsigned char *buf, unsigned int nbytes, unsigned long *offs,
>>> -     uint32_t *checksum, void *dgst);
>>>  int openfile(const char *filename);
>>>  int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs,
>>>       unsigned long long seek,
>>> @@ -167,22 +165,6 @@ int get_install_info(sourcetype *source, char *buf, size_t len);
>>>
>>>  unsigned long long ustrtoull(const char *cp, char **endp, unsigned int base);
>>>
>>> -#ifdef CONFIG_GUNZIP
>>> -int decompress_image(int infile, unsigned long *offs, int nbytes,
>>> -     int outfile, uint32_t *checksum, void *dgst);
>>> -#else
>>> -static inline int decompress_image(int __attribute__ ((__unused__))infile,
>>> -                unsigned long __attribute__ ((__unused__)) *offs,
>>> -                int __attribute__ ((__unused__)) nbytes,
>>> -                int __attribute__ ((__unused__)) outfile,
>>> -                uint32_t __attribute__ ((__unused__)) *checksum,
>>> -                void __attribute__ ((__unused__)) *dgst) {
>>> -
>>> -             TRACE("Request decompressing, but CONFIG_GUNZIP not set !");
>>> -     return -1;
>>> -}
>>> -#endif
>>> -
>>>  const char* get_tmpdir(void);
>>>  const char* get_tmpdirscripts(void);
>>>
>>>
>>
>> My bigger concern is that the patch is replacing a central piece inside
>> SWUpdate. Can you ask which kind of tests you have already done and
>> which regressions tests ?
> 
> I understand, this is indeed a significant change and I won't take it
> bad if you run your own tests before submitting my patch.

Nevertheless, I agree to improve code and to make it cleaner. And
removing gun.c (take as it is with small changes from original one) is a
step in the right direction. I want just to be sure that I am not
breaking the behavior.

> I successfully compiled swupdate with and without gzip support.
> I tested the modified code on real hardware with the following configurations:
> - Digi ccimx6 (eMMC GPT partitions, raw ext4 images):
>   - no compression, no encryption
>   - compression only
>   - encryption only
>   - encryption+compression

ok, fine

> - Digi ccimx6ul (NAND partitions, UBI volumes, UBIFS images):
>   - no compression, no encryption
>   - encryption only
> In UBIFS images file data is normally already Zlib- or LZO-compressed,

It is - it makes no sense to compress UBIFS images.

> so I have not tested the scenarios that involved gzip compression.

ok - anyway, SWUpdate works with "pipelines", too, and because
copyfile() is used by quite all handlers, replacing this code should be
independent from the handler itself.

> I have not conducted any performance check (execution time, RAM usage).

Nevermind. I do not expect changes in RAM usage: decompression /
decryption is done per chunk, and gun.c is a user of zlib, too. My
expectation is that RAM usage is quite the same.

Best regards,
Stefano Babic

Patch
diff mbox series

diff --git a/Makefile b/Makefile
index 812a8a4..11fc2f5 100644
--- a/Makefile
+++ b/Makefile
@@ -343,7 +343,7 @@  include $(srctree)/Makefile.flags
 # Defaults to vmlinux, but the arch makefile usually adds further targets
 
 objs-y		:= core handlers
-libs-y		:= archival corelib ipc mongoose parser suricatta bootloader
+libs-y		:= corelib ipc mongoose parser suricatta bootloader
 tools-y		:= tools
 
 swupdate-dirs	:= $(objs-y) $(libs-y)
diff --git a/archival/Makefile b/archival/Makefile
deleted file mode 100644
index 5715d0b..0000000
--- a/archival/Makefile
+++ /dev/null
@@ -1 +0,0 @@ 
-lib-$(CONFIG_GUNZIP)		+= gun.o
diff --git a/archival/gun.c b/archival/gun.c
deleted file mode 100644
index ade0379..0000000
--- a/archival/gun.c
+++ /dev/null
@@ -1,519 +0,0 @@ 
-/* gun.c -- simple gunzip to give an example of the use of inflateBack()
- * Copyright (C) 2003, 2005 Mark Adler
- * For conditions of distribution and use, see copyright notice in zlib.h
-   Version 1.3  12 June 2005  Mark Adler */
-
-/* Version history:
-   1.0  16 Feb 2003  First version for testing of inflateBack()
-   1.1  21 Feb 2005  Decompress concatenated gzip streams
-                     Remove use of "this" variable (C++ keyword)
-                     Fix return value for in()
-                     Improve allocation failure checking
-                     Add typecasting for void * structures
-                     Add -h option for command version and usage
-                     Add a bunch of comments
-   1.2  20 Mar 2005  Add Unix compress (LZW) decompression
-                     Copy file attributes from input file to output file
-   1.3  12 Jun 2005  Add casts for error messages [Oberhumer]
- */
-
-/* external functions and related types and constants */
-#include <stdio.h>          /* fprintf() */
-#include <stdlib.h>         /* malloc(), free() */
-#include <string.h>         /* strerror(), strcmp(), strlen(), memcpy() */
-#include <errno.h>          /* errno */
-#include <fcntl.h>          /* open() */
-#include <unistd.h>         /* read(), write(), close(), chown(), unlink() */
-#include <sys/types.h>
-#include <sys/stat.h>       /* stat(), chmod() */
-#include <utime.h>          /* utime() */
-#include <zlib.h>           /* inflateBackInit(), inflateBack(), */
-                            /* inflateBackEnd(), crc32() */
-#include <util.h>
-#include <progress.h>
-
-/* buffer constants */
-#define SIZE 32768U         /* input and output buffer sizes */
-#define PIECE 16384         /* limits i/o chunks for 16-bit int case */
-
-#define MODULE_NAME "gunzip"
-
-/* structure for infback() to pass to input function in() -- it maintains the
-   input file and a buffer of size SIZE */
-struct ind {
-    int infile;
-    unsigned char *inbuf;
-    unsigned long *offs;
-    unsigned long *checksum;
-    int nbytes;
-    int total;
-    int percent;
-    void *dgst;
-};
-
-/* Load input buffer, assumed to be empty, and return bytes loaded and a
-   pointer to them.  read() is called until the buffer is full, or until it
-   returns end-of-file or error.  Return 0 on error. */
-static unsigned in(void *in_desc, unsigned char **buf)
-{
-    int ret;
-    unsigned len;
-    unsigned char *next;
-    struct ind *me = (struct ind *)in_desc;
-    unsigned int percent;
-
-    next = me->inbuf;
-    *buf = next;
-    len = 0;
-    do {
-        ret = PIECE;
-        if ((unsigned)ret > SIZE - len)
-            ret = (int)(SIZE - len);
-	if (ret > me->nbytes)
-		ret = me->nbytes;
-	ret = fill_buffer(me->infile, next, ret, me->offs, (uint32_t *)me->checksum, me->dgst);
-        if (ret < 0) {
-            len = 0;
-            break;
-        }
-        next += ret;
-        len += ret;
-	me->nbytes -= ret;
-    } while (ret != 0 && len < SIZE);
-    percent = (unsigned int)(((double)(me->total - me->nbytes)) * 100 /
-		    (me->total ? me->total : 1));
-    if (percent != (unsigned int)me->percent) {
-	me->percent = percent;
-	swupdate_progress_update(percent);
-    }
-    return len;
-}
-
-/* structure for infback() to pass to output function out() -- it maintains the
-   output file, a running CRC-32 check on the output and the total number of
-   bytes output, both for checking against the gzip trailer.  (The length in
-   the gzip trailer is stored modulo 2^32, so it's ok if a long is 32 bits and
-   the output is greater than 4 GB.) */
-struct outd {
-    int outfile;
-    int check;                  /* true if checking crc and total */
-    unsigned long crc;
-    unsigned long total;
-};
-
-/* Write output buffer and update the CRC-32 and total bytes written.  write()
-   is called until all of the output is written or an error is encountered.
-   On success out() returns 0.  For a write failure, out() returns 1.  If the
-   output file descriptor is -1, then nothing is written.
- */
-static int out(void *out_desc, unsigned char *buf, unsigned len)
-{
-    int ret;
-    struct outd *me = (struct outd *)out_desc;
-
-    if (me->check) {
-        me->crc = crc32(me->crc, buf, len);
-        me->total += len;
-    }
-    if (me->outfile != -1)
-        do {
-            ret = PIECE;
-            if ((unsigned)ret > len)
-                ret = (int)len;
-            ret = (int)write(me->outfile, buf, ret);
-            if (ret == -1)
-                return 1;
-            buf += ret;
-            len -= ret;
-        } while (len != 0);
-    return 0;
-}
-
-/* next input byte macro for use inside lunpipe() and gunpipe() */
-#define NEXT() (have ? 0 : (have = in(indp, &next)), \
-                last = have ? (have--, (int)(*next++)) : -1)
-
-/* memory for gunpipe() and lunpipe() --
-   the first 256 entries of prefix[] and suffix[] are never used, could
-   have offset the index, but it's faster to waste the memory */
-unsigned char inbuf[SIZE];              /* input buffer */
-unsigned char outbuf[SIZE];             /* output buffer */
-unsigned short prefix[65536];           /* index to LZW prefix string */
-unsigned char suffix[65536];            /* one-character LZW suffix */
-unsigned char match[65280 + 2];         /* buffer for reversed match or gzip
-                                           32K sliding window */
-
-/* throw out what's left in the current bits byte buffer (this is a vestigial
-   aspect of the compressed data format derived from an implementation that
-   made use of a special VAX machine instruction!) */
-#define FLUSHCODE() \
-    do { \
-        left = 0; \
-        rem = 0; \
-        if (chunk > have) { \
-            chunk -= have; \
-            have = 0; \
-            if (NEXT() == -1) \
-                break; \
-            chunk--; \
-            if (chunk > have) { \
-                chunk = have = 0; \
-                break; \
-            } \
-        } \
-        have -= chunk; \
-        next += chunk; \
-        chunk = 0; \
-    } while (0)
-
-/* Decompress a compress (LZW) file from indp to outfile.  The compress magic
-   header (two bytes) has already been read and verified.  There are have bytes
-   of buffered input at next.  strm is used for passing error information back
-   to gunpipe().
-
-   lunpipe() will return Z_OK on success, Z_BUF_ERROR for an unexpected end of
-   file, read error, or write error (a write error indicated by strm->next_in
-   not equal to Z_NULL), or Z_DATA_ERROR for invalid input.
- */
-static int lunpipe(unsigned have, unsigned char *next, struct ind *indp,
-                  int outfile, z_stream *strm)
-{
-    int last;                   /* last byte read by NEXT(), or -1 if EOF */
-    unsigned chunk;             /* bytes left in current chunk */
-    int left;                   /* bits left in rem */
-    unsigned rem;               /* unused bits from input */
-    int bits;                   /* current bits per code */
-    unsigned code;              /* code, table traversal index */
-    unsigned mask;              /* mask for current bits codes */
-    int max;                    /* maximum bits per code for this stream */
-    int flags;                  /* compress flags, then block compress flag */
-    unsigned end;               /* last valid entry in prefix/suffix tables */
-    unsigned temp;              /* current code */
-    unsigned prev;              /* previous code */
-    unsigned final;             /* last character written for previous code */
-    unsigned stack;             /* next position for reversed string */
-    unsigned outcnt;            /* bytes in output buffer */
-    struct outd outd;           /* output structure */
-
-    /* set up output */
-    outd.outfile = outfile;
-    outd.check = 0;
-
-    /* process remainder of compress header -- a flags byte */
-    flags = NEXT();
-    if (last == -1)
-        return Z_BUF_ERROR;
-    if (flags & 0x60) {
-        strm->msg = (char *)"unknown lzw flags set";
-        return Z_DATA_ERROR;
-    }
-    max = flags & 0x1f;
-    if (max < 9 || max > 16) {
-        strm->msg = (char *)"lzw bits out of range";
-        return Z_DATA_ERROR;
-    }
-    if (max == 9)                           /* 9 doesn't really mean 9 */
-        max = 10;
-    flags &= 0x80;                          /* true if block compress */
-
-    /* clear table */
-    bits = 9;
-    mask = 0x1ff;
-    end = flags ? 256 : 255;
-
-    /* set up: get first 9-bit code, which is the first decompressed byte, but
-       don't create a table entry until the next code */
-    if (NEXT() == -1)                       /* no compressed data is ok */
-        return Z_OK;
-    final = prev = (unsigned)last;          /* low 8 bits of code */
-    if (NEXT() == -1)                       /* missing a bit */
-        return Z_BUF_ERROR;
-    if (last & 1) {                         /* code must be < 256 */
-        strm->msg = (char *)"invalid lzw code";
-        return Z_DATA_ERROR;
-    }
-    rem = (unsigned)last >> 1;              /* remaining 7 bits */
-    left = 7;
-    chunk = bits - 2;                       /* 7 bytes left in this chunk */
-    outbuf[0] = (unsigned char)final;       /* write first decompressed byte */
-    outcnt = 1;
-
-    /* decode codes */
-    stack = 0;
-    for (;;) {
-        /* if the table will be full after this, increment the code size */
-        if (end >= mask && bits < max) {
-            FLUSHCODE();
-            bits++;
-            mask <<= 1;
-            mask++;
-        }
-
-        /* get a code of length bits */
-        if (chunk == 0)                     /* decrement chunk modulo bits */
-            chunk = bits;
-        code = rem;                         /* low bits of code */
-        if (NEXT() == -1) {                 /* EOF is end of compressed data */
-            /* write remaining buffered output */
-            if (outcnt && out(&outd, outbuf, outcnt)) {
-                strm->next_in = outbuf;     /* signal write error */
-                return Z_BUF_ERROR;
-            }
-            return Z_OK;
-        }
-        code += (unsigned)last << left;     /* middle (or high) bits of code */
-        left += 8;
-        chunk--;
-        if (bits > left) {                  /* need more bits */
-            if (NEXT() == -1)               /* can't end in middle of code */
-                return Z_BUF_ERROR;
-            code += (unsigned)last << left; /* high bits of code */
-            left += 8;
-            chunk--;
-        }
-        code &= mask;                       /* mask to current code length */
-        left -= bits;                       /* number of unused bits */
-        rem = (unsigned)last >> (8 - left); /* unused bits from last byte */
-
-        /* process clear code (256) */
-        if (code == 256 && flags) {
-            FLUSHCODE();
-            bits = 9;                       /* initialize bits and mask */
-            mask = 0x1ff;
-            end = 255;                      /* empty table */
-            continue;                       /* get next code */
-        }
-
-        /* special code to reuse last match */
-        temp = code;                        /* save the current code */
-        if (code > end) {
-            /* Be picky on the allowed code here, and make sure that the code
-               we drop through (prev) will be a valid index so that random
-               input does not cause an exception.  The code != end + 1 check is
-               empirically derived, and not checked in the original uncompress
-               code.  If this ever causes a problem, that check could be safely
-               removed.  Leaving this check in greatly improves gun's ability
-               to detect random or corrupted input after a compress header.
-               In any case, the prev > end check must be retained. */
-            if (code != end + 1 || prev > end) {
-                strm->msg = (char *)"invalid lzw code";
-                return Z_DATA_ERROR;
-            }
-            match[stack++] = (unsigned char)final;
-            code = prev;
-        }
-
-        /* walk through linked list to generate output in reverse order */
-        while (code >= 256) {
-            match[stack++] = suffix[code];
-            code = prefix[code];
-        }
-        match[stack++] = (unsigned char)code;
-        final = code;
-
-        /* link new table entry */
-        if (end < mask) {
-            end++;
-            prefix[end] = (unsigned short)prev;
-            suffix[end] = (unsigned char)final;
-        }
-
-        /* set previous code for next iteration */
-        prev = temp;
-
-        /* write output in forward order */
-        while (stack > SIZE - outcnt) {
-            while (outcnt < SIZE)
-                outbuf[outcnt++] = match[--stack];
-            if (out(&outd, outbuf, outcnt)) {
-                strm->next_in = outbuf; /* signal write error */
-                return Z_BUF_ERROR;
-            }
-            outcnt = 0;
-        }
-        do {
-            outbuf[outcnt++] = match[--stack];
-        } while (stack);
-
-        /* loop for next code with final and prev as the last match, rem and
-           left provide the first 0..7 bits of the next code, end is the last
-           valid table entry */
-    }
-}
-
-
-/* Decompress a gzip file from infile to outfile.  strm is assumed to have been
-   successfully initialized with inflateBackInit().  The input file may consist
-   of a series of gzip streams, in which case all of them will be decompressed
-   to the output file.  If outfile is -1, then the gzip stream(s) integrity is
-   checked and nothing is written.
-
-   The return value is a zlib error code: Z_MEM_ERROR if out of memory,
-   Z_DATA_ERROR if the header or the compressed data is invalid, or if the
-   trailer CRC-32 check or length doesn't match, Z_BUF_ERROR if the input ends
-   prematurely or a write error occurs, or Z_ERRNO if junk (not a another gzip
-   stream) follows a valid gzip stream.
- */
-static int gunpipe(z_stream *strm, int infile, unsigned long *offs, int nbytes, int outfile, uint32_t *checksum, void *dgst)
-{
-    int ret, first, last;
-    unsigned have, flags, len;
-    unsigned char *next;
-    struct ind ind, *indp;
-    struct outd outd;
-
-    /* setup input buffer */
-    ind.infile = infile;
-    ind.inbuf = inbuf;
-    ind.nbytes = nbytes;
-    ind.total = nbytes; /* Used just for progress */
-    ind.percent = 0;
-    ind.offs = offs;
-    ind.checksum = (unsigned long *)checksum;
-    ind.dgst = dgst; /* digest for computing hashes */
-    indp = &ind;
-
-    /* decompress concatenated gzip streams */
-    have = 0;                               /* no input data read in yet */
-    first = 1;                              /* looking for first gzip header */
-    strm->next_in = Z_NULL;                 /* so Z_BUF_ERROR means EOF */
-    for (;;) {
-        /* look for the two magic header bytes for a gzip stream */
-        if (NEXT() == -1) {
-            ret = Z_OK;
-            break;                          /* empty gzip stream is ok */
-        }
-        if (last != 31 || (NEXT() != 139 && last != 157)) {
-            strm->msg = (char *)"incorrect header check";
-            ret = first ? Z_DATA_ERROR : Z_ERRNO;
-            break;                          /* not a gzip or compress header */
-        }
-        first = 0;                          /* next non-header is junk */
-
-        /* process a compress (LZW) file -- can't be concatenated after this */
-        if (last == 157) {
-            ret = lunpipe(have, next, indp, outfile, strm);
-            break;
-        }
-
-        /* process remainder of gzip header */
-        ret = Z_BUF_ERROR;
-        if (NEXT() != 8) {                  /* only deflate method allowed */
-            if (last == -1) break;
-            strm->msg = (char *)"unknown compression method";
-            ret = Z_DATA_ERROR;
-            break;
-        }
-        flags = NEXT();                     /* header flags */
-        NEXT();                             /* discard mod time, xflgs, os */
-        NEXT();
-        NEXT();
-        NEXT();
-        NEXT();
-        NEXT();
-        if (last == -1) break;
-        if (flags & 0xe0) {
-            strm->msg = (char *)"unknown header flags set";
-            ret = Z_DATA_ERROR;
-            break;
-        }
-        if (flags & 4) {                    /* extra field */
-            len = NEXT();
-            len += (unsigned)(NEXT()) << 8;
-            if (last == -1) break;
-            while (len > have) {
-                len -= have;
-                have = 0;
-                if (NEXT() == -1) break;
-                len--;
-            }
-            if (last == -1) break;
-            have -= len;
-            next += len;
-        }
-        if (flags & 8)                      /* file name */
-            while (NEXT() != 0 && last != -1)
-                ;
-        if (flags & 16)                     /* comment */
-            while (NEXT() != 0 && last != -1)
-                ;
-        if (flags & 2) {                    /* header crc */
-            NEXT();
-            NEXT();
-        }
-        if (last == -1) break;
-
-        /* set up output */
-        outd.outfile = outfile;
-        outd.check = 1;
-        outd.crc = crc32(0L, Z_NULL, 0);
-        outd.total = 0;
-
-        /* decompress data to output */
-        strm->next_in = next;
-        strm->avail_in = have;
-        ret = inflateBack(strm, in, indp, out, &outd);
-        if (ret != Z_STREAM_END) break;
-        next = strm->next_in;
-        have = strm->avail_in;
-        strm->next_in = Z_NULL;             /* so Z_BUF_ERROR means EOF */
-
-        /* check trailer */
-        ret = Z_BUF_ERROR;
-        if (NEXT() != (int)(outd.crc & 0xff) ||
-            NEXT() != (int)((outd.crc >> 8) & 0xff) ||
-            NEXT() != (int)((outd.crc >> 16) & 0xff) ||
-            NEXT() != (int)((outd.crc >> 24) & 0xff)) {
-            /* crc error */
-            if (last != -1) {
-                strm->msg = (char *)"incorrect data check";
-                ret = Z_DATA_ERROR;
-            }
-            break;
-        }
-        if (NEXT() != (int)(outd.total & 0xff) ||
-            NEXT() != (int)((outd.total >> 8) & 0xff) ||
-            NEXT() != (int)((outd.total >> 16) & 0xff) ||
-            NEXT() != (int)((outd.total >> 24) & 0xff)) {
-            /* length error */
-            if (last != -1) {
-                strm->msg = (char *)"incorrect length check";
-                ret = Z_DATA_ERROR;
-            }
-            break;
-        }
-
-        /* go back and look for another gzip stream */
-    }
-
-    /* clean up and return */
-    return ret;
-}
-
-/* Process the gun command line arguments.  See the command syntax near the
-   beginning of this source file. */
-int decompress_image(int infile, unsigned long *offs, int nbytes,
-	int outfile, uint32_t *checksum, void *dgst)
-{
-    int ret;
-    unsigned char *window;
-    z_stream strm;
-
-    *checksum = 0;
-
-    /* initialize inflateBack state for repeated use */
-    window = match;                         /* reuse LZW match buffer */
-    strm.zalloc = Z_NULL;
-    strm.zfree = Z_NULL;
-    strm.opaque = Z_NULL;
-    ret = inflateBackInit(&strm, 15, window);
-    if (ret != Z_OK) {
-        ERROR("gun out of memory error--aborting\n");
-        return 1;
-    }
-    errno = 0;
-    ret = gunpipe(&strm, infile, offs, nbytes, outfile, checksum, dgst);
-    /* clean up */
-    inflateBackEnd(&strm);
-    return ret;
-}
diff --git a/core/cpio_utils.c b/core/cpio_utils.c
index 85f3fc0..26f155c 100644
--- a/core/cpio_utils.c
+++ b/core/cpio_utils.c
@@ -5,10 +5,14 @@ 
  * SPDX-License-Identifier:     GPL-2.0-or-later
  */
 
+#include <stdbool.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <unistd.h>
 #include <errno.h>
+#ifdef CONFIG_GUNZIP
+#include <zlib.h>
+#endif
 
 #include "generated/autoconf.h"
 #include "cpiohdr.h"
@@ -39,7 +43,7 @@  static int get_cpiohdr(unsigned char *buf, unsigned long *size,
 	return 0;
 }
 
-int fill_buffer(int fd, unsigned char *buf, unsigned int nbytes, unsigned long *offs,
+static int fill_buffer(int fd, unsigned char *buf, unsigned int nbytes, unsigned long *offs,
 	uint32_t *checksum, void *dgst)
 {
 	ssize_t len;
@@ -99,72 +103,233 @@  static int copy_write(void *out, const void *buf, unsigned int len)
 	return 0;
 }
 
+typedef int (*PipelineStep)(void *state, void *buffer, size_t size);
+
+struct InputState
+{
+	int fdin;
+	unsigned int nbytes;
+	unsigned long *offs;
+	void *dgst;	/* use a private context for HASH */
+	uint32_t checksum;
+};
+
+static int input_step(void *state, void *buffer, size_t size)
+{
+	struct InputState *s = (struct InputState *)state;
+	if (size >= s->nbytes) {
+		size = s->nbytes;
+	}
+	int ret = fill_buffer(s->fdin, buffer, size, s->offs, &s->checksum, s->dgst);
+	if (ret < 0) {
+		return ret;
+	}
+	s->nbytes -= size;
+	return ret;
+}
+
+struct DecryptState
+{
+	PipelineStep upstream_step;
+	void *upstream_state;
+
+	void *dcrypt;	/* use a private context for decryption */
+	uint8_t input[BUFF_SIZE];
+	uint8_t output[BUFF_SIZE + AES_BLOCK_SIZE];
+	int outlen;
+	bool eof;
+};
+
+static int decrypt_step(void *state, void *buffer, size_t size)
+{
+	struct DecryptState *s = (struct DecryptState *)state;
+	int ret;
+	int inlen;
+
+	if (s->outlen != 0) {
+		if (size > s->outlen) {
+			size = s->outlen;
+		}
+		memcpy(buffer, s->output, size);
+		s->outlen -= size;
+		memmove(s->output, s->output + size, s->outlen);
+		return size;
+	}
+
+	ret = s->upstream_step(s->upstream_state, s->input, sizeof s->input);
+	if (ret < 0) {
+		return ret;
+	}
+
+	inlen = ret;
+
+	if (!s->eof) {
+		if (inlen != 0) {
+			ret = swupdate_DECRYPT_update(s->dcrypt,
+				s->output, &s->outlen, s->input, inlen);
+		} else {
+			/*
+			 * Finalise the decryption. Further plaintext bytes may be written at
+			 * this stage.
+			 */
+			ret = swupdate_DECRYPT_final(s->dcrypt,
+				s->output, &s->outlen);
+			s->eof = true;
+		}
+		if (ret < 0) {
+			return ret;
+		}
+	}
+
+	if (s->outlen != 0) {
+		if (size > s->outlen) {
+			size = s->outlen;
+		}
+		memcpy(buffer, s->output, size);
+		s->outlen -= size;
+		memmove(s->output, s->output + size, s->outlen);
+		return size;
+	}
+
+	return 0;
+}
+
+#ifdef CONFIG_GUNZIP
+
+struct GunzipState
+{
+	PipelineStep upstream_step;
+	void *upstream_state;
+
+	z_stream strm;
+	bool initialized;
+	uint8_t input[BUFF_SIZE];
+	bool eof;
+};
+
+static int gunzip_step(void *state, void *buffer, size_t size)
+{
+	struct GunzipState *s = (struct GunzipState *)state;
+	int ret;
+	int outlen = 0;
+
+	s->strm.next_out = buffer;
+	s->strm.avail_out = size;
+	while (outlen == 0) {
+		if (s->strm.avail_in == 0) {
+			ret = s->upstream_step(s->upstream_state, s->input, sizeof s->input);
+			if (ret < 0) {
+				return ret;
+			}
+			s->strm.avail_in = ret;
+			s->strm.next_in = s->input;
+		}
+		if (s->eof) {
+			break;
+		}
+
+		ret = inflate(&s->strm, Z_NO_FLUSH);
+		outlen = size - s->strm.avail_out;
+		if (ret == Z_STREAM_END) {
+			s->eof = true;
+			break;
+		}
+		if (ret != Z_OK && ret != Z_BUF_ERROR) {
+			ERROR("inflate failed (returned %d)", ret);
+			return -1;
+		}
+	}
+	return outlen;
+}
+
+#endif
+
 int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs, unsigned long long seek,
 	int skip_file, int __attribute__ ((__unused__)) compressed,
 	uint32_t *checksum, unsigned char *hash, int encrypted, writeimage callback)
 {
-	unsigned long size;
-	unsigned char *in = NULL, *inbuf;
-	unsigned char *decbuf = NULL;
-	unsigned long filesize = nbytes;
 	unsigned int percent, prevpercent = 0;
 	int ret = 0;
-	void *dgst = NULL;	/* use a private context for HASH */
-	void *dcrypt = NULL;	/* use a private context for decryption */
 	int len;
 	unsigned char md_value[64]; /*
 				     *  Maximum hash is 64 bytes for SHA512
 				     *  and we use sha256 in swupdate
 				     */
 	unsigned int md_len = 0;
-	unsigned char *aes_key;
-	unsigned char *ivt;
+	unsigned char *aes_key = NULL;
+	unsigned char *ivt = NULL;
 	unsigned char *salt;
 
+	struct InputState input_state = {
+		.fdin = fdin,
+		.nbytes = nbytes,
+		.offs = offs,
+		.dgst = NULL,
+		.checksum = 0
+	};
+
+	struct DecryptState decrypt_state = {
+		.upstream_step = NULL, .upstream_state = NULL,
+		.dcrypt = NULL,
+		.outlen = 0, .eof = false
+	};
+
+#ifdef CONFIG_GUNZIP
+	struct GunzipState gunzip_state = {
+		.upstream_step = NULL, .upstream_state = NULL,
+		.strm = {
+			.zalloc = Z_NULL, .zfree = Z_NULL, .opaque = Z_NULL,
+			.avail_in = 0, .next_in = Z_NULL,
+			.avail_out = 0, .next_out = Z_NULL
+		},
+		.initialized = false,
+		.eof = false
+	};
+#endif
+
+	PipelineStep step = NULL;
+	void *state = NULL;
+	uint8_t buffer[BUFF_SIZE];
+
 	if (!callback) {
 		callback = copy_write;
 	}
 
-	if (IsValidHash(hash)) {
-		dgst = swupdate_HASH_init(SHA_DEFAULT);
-		if (!dgst)
-			return -EFAULT;
-	}
-
 	if (checksum)
 		*checksum = 0;
 
-	/*
-	 * Simultaneous compression and decryption of images
-	 * is not (yet ?) supported
-	 */
-	if (compressed && encrypted) {
-		ERROR("encrypted zip images are not yet supported -- aborting\n");
-		return -EINVAL;
+	if (IsValidHash(hash)) {
+		input_state.dgst = swupdate_HASH_init(SHA_DEFAULT);
+		if (!input_state.dgst)
+			return -EFAULT;
 	}
-
-	in = (unsigned char *)malloc(BUFF_SIZE);
-	if (!in)
-		return -ENOMEM;
-
+ 
 	if (encrypted) {
-		decbuf = (unsigned char *)calloc(1, BUFF_SIZE + AES_BLOCK_SIZE);
-		if (!decbuf) {
-			ret = -ENOMEM;
-			goto copyfile_exit;
-		}
-
 		aes_key = get_aes_key();
 		ivt = get_aes_ivt();
 		salt = get_aes_salt();
-		dcrypt = swupdate_DECRYPT_init(aes_key, ivt, salt);
-		if (!dcrypt) {
+		decrypt_state.dcrypt = swupdate_DECRYPT_init(aes_key, ivt, salt);
+		if (!decrypt_state.dcrypt) {
 			ERROR("decrypt initialization failure, aborting");
 			ret = -EFAULT;
 			goto copyfile_exit;
 		}
 	}
 
+	if (compressed) {
+#ifdef CONFIG_GUNZIP
+		if (inflateInit2(&gunzip_state.strm, 16 + MAX_WBITS) != Z_OK) {
+			ERROR("inflateInit2 failed");
+			ret = -EFAULT;
+			goto copyfile_exit;
+		}
+		gunzip_state.initialized = true;
+#else
+		TRACE("Request decompressing, but CONFIG_GUNZIP not set !");
+		return -1;
+#endif
+	}
+
 	if (seek) {
 		int fdout = (out != NULL) ? *(int *)out : -1;
 		TRACE("offset has been defined: %llu bytes\n", seek);
@@ -175,74 +340,66 @@  int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs, unsi
 		}
 	}
 
-	int fdout = (out != NULL) ? *(int *)out : -1;
+#ifdef CONFIG_GUNZIP
 	if (compressed) {
-		ret = decompress_image(fdin, offs, nbytes, fdout, checksum, dgst);
-		if (ret < 0) {
-			ERROR("gunzip failure %d (errno %d) -- aborting\n", ret, errno);
-			goto copyfile_exit;
+		if (encrypted) {
+			decrypt_state.upstream_step = &input_step;
+			decrypt_state.upstream_state = &input_state;
+			gunzip_state.upstream_step = &decrypt_step;
+			gunzip_state.upstream_state = &decrypt_state;
+		} else {
+			gunzip_state.upstream_step = &input_step;
+			gunzip_state.upstream_state = &input_state;
 		}
+		step = &gunzip_step;
+		state = &gunzip_state;
 	} else {
+#endif
+		if (encrypted) {
+			decrypt_state.upstream_step = &input_step;
+			decrypt_state.upstream_state = &input_state;
+			step = &decrypt_step;
+			state = &decrypt_state;
+		} else {
+			step = &input_step;
+			state = &input_state;
+		}
+#ifdef CONFIG_GUNZIP
+	}
+#endif
 
-	while (nbytes > 0) {
-		size = (nbytes < BUFF_SIZE ? nbytes : BUFF_SIZE);
-
-		if ((ret = fill_buffer(fdin, in, size, offs, checksum, dgst) < 0)) {
+	for (;;) {
+		ret = step(state, buffer, sizeof buffer);
+		if (ret < 0) {
 			goto copyfile_exit;
 		}
-
-		nbytes -= size;
-		if (skip_file)
+		if (ret == 0) {
+			break;
+		}
+		if (skip_file) {
 			continue;
-
-		inbuf = in;
-		len = size;
-
-		if (encrypted) {
-			ret = swupdate_DECRYPT_update(dcrypt, decbuf,
-				&len, in, size);
-			if (ret < 0)
-				goto copyfile_exit;
-			inbuf = decbuf;
 		}
-
+		len = ret;
 		/*
 		 * If there is no enough place,
 		 * returns an error and close the output file that
 		 * results corrupted. This lets the cleanup routine
 		 * to remove it
 		 */
-		if (callback(out, inbuf, len) < 0) {
-			ret =-ENOSPC;
+		if (callback(out, buffer, len) < 0) {
+			ret = -ENOSPC;
 			goto copyfile_exit;
 		}
 
-		percent = (unsigned int)(((double)(filesize - nbytes)) * 100 / filesize);
+		percent = (unsigned)(100ULL * (nbytes - input_state.nbytes) / nbytes);
 		if (percent != prevpercent) {
 			prevpercent = percent;
 			swupdate_progress_update(percent);
 		}
 	}
 
-	}
-
-	/*
-	 * Finalise the decryption. Further plaintext bytes may be written at
-	 * this stage.
-	 */
-	if (encrypted) {
-		ret = swupdate_DECRYPT_final(dcrypt, decbuf, &len);
-		if (ret < 0)
-			goto copyfile_exit;
-		if (callback(out, decbuf, len) < 0) {
-			ret =-ENOSPC;
-			goto copyfile_exit;
-		}
-	}
-
-
 	if (IsValidHash(hash)) {
-		if (swupdate_HASH_final(dgst, md_value, &md_len) < 0) {
+		if (swupdate_HASH_final(input_state.dgst, md_value, &md_len) < 0) {
 			ret = -EFAULT;
 			goto copyfile_exit;
 		}
@@ -266,21 +423,26 @@  int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs, unsi
 		}
 	}
 
-	fill_buffer(fdin, in, NPAD_BYTES(*offs), offs, checksum, NULL);
+	fill_buffer(fdin, buffer, NPAD_BYTES(*offs), offs, checksum, NULL);
+
+	if (checksum != NULL) {
+		*checksum = input_state.checksum;
+	}
 
 	ret = 0;
 
 copyfile_exit:
-	if (in)
-		free(in);
-	if (dcrypt) {
-		swupdate_DECRYPT_cleanup(dcrypt);
+	if (decrypt_state.dcrypt) {
+		swupdate_DECRYPT_cleanup(decrypt_state.dcrypt);
+	}
+	if (input_state.dgst) {
+		swupdate_HASH_cleanup(input_state.dgst);
 	}
-	if (decbuf)
-		free(decbuf);
-	if (dgst) {
-		swupdate_HASH_cleanup(dgst);
+#ifdef CONFIG_GUNZIP
+	if (gunzip_state.initialized) {
+		inflateEnd(&gunzip_state.strm);
 	}
+#endif
 
 	return ret;
 }
diff --git a/doc/source/roadmap.rst b/doc/source/roadmap.rst
index 0502cbe..bc43b9f 100644
--- a/doc/source/roadmap.rst
+++ b/doc/source/roadmap.rst
@@ -108,11 +108,6 @@  Current release supports verified images. That means that a handler
 written in Lua could be now be part of the compound image, because
 a unauthenticated handler cannot run.
 
-Encryption of compressed artifacts
-==================================
-
-Currently, encrypted artifact are not compressed. Allow to compress artifacts before encryption.
-
 Support for evaluation boards
 =============================
 
diff --git a/handlers/ubivol_handler.c b/handlers/ubivol_handler.c
index 25ac837..2c4f95e 100644
--- a/handlers/ubivol_handler.c
+++ b/handlers/ubivol_handler.c
@@ -19,6 +19,7 @@ 
 #include "handler.h"
 #include "flash.h"
 #include "util.h"
+#include "sslapi.h"
 
 void ubi_handler(void);
 
@@ -43,6 +44,9 @@  static int update_volume(libubi_t libubi, struct img_type *img,
 	char sbuf[128];
 
 	bytes = img->size;
+	if (img->is_encrypted) {
+		bytes -= AES_BLOCK_SIZE;
+	}
 
 	if (!libubi) {
 		ERROR("Request to write into UBI, but no UBI on system");
diff --git a/include/util.h b/include/util.h
index d43cd8c..bff9436 100644
--- a/include/util.h
+++ b/include/util.h
@@ -130,8 +130,6 @@  char *sdup(const char *str);
  */
 typedef int (*writeimage) (void *out, const void *buf, unsigned int len);
 
-int fill_buffer(int fd, unsigned char *buf, unsigned int nbytes, unsigned long *offs,
-	uint32_t *checksum, void *dgst);
 int openfile(const char *filename);
 int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs,
 	unsigned long long seek,
@@ -167,22 +165,6 @@  int get_install_info(sourcetype *source, char *buf, size_t len);
 
 unsigned long long ustrtoull(const char *cp, char **endp, unsigned int base);
 
-#ifdef CONFIG_GUNZIP
-int decompress_image(int infile, unsigned long *offs, int nbytes,
-	int outfile, uint32_t *checksum, void *dgst);
-#else
-static inline int decompress_image(int __attribute__ ((__unused__))infile,
-		   unsigned long __attribute__ ((__unused__)) *offs,
-		   int __attribute__ ((__unused__)) nbytes,
-		   int __attribute__ ((__unused__)) outfile,
-		   uint32_t __attribute__ ((__unused__)) *checksum,
-		   void __attribute__ ((__unused__)) *dgst) {
-
-		TRACE("Request decompressing, but CONFIG_GUNZIP not set !");
-	return -1;
-}
-#endif
-
 const char* get_tmpdir(void);
 const char* get_tmpdirscripts(void);