Encryption of ubi volume image

Message ID 1528452723-581-1-git-send-email-angelo@amarulasolutions.com
State New
Headers show
Series
  • Encryption of ubi volume image
Related show

Commit Message

Angelo Compagnucci June 8, 2018, 10:12 a.m.
Encryption on ubi volumes is actually broken cause a mismatch between
the size expected to be written and the actual size after decrypting.
Actually, the size to be written on disk is retrieved from the size of
the encrypted image when the real size of a decrypted image is actually
smaller.

This patch adds a "decrypted-size" attribute to the sw-description to
explicitly tell to swupdate to allocate that size for ubi volumes.
This parameter should be updated to the real size of the image just
before assembling the update.

Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
---
 handlers/ubivol_handler.c |  3 ++-
 include/swupdate.h        |  1 +
 parser/parse_external.c   | 16 ++++++++++++++++
 parser/parser.c           | 15 +++++++++++++++
 4 files changed, 34 insertions(+), 1 deletion(-)

Comments

Stefano Babic June 13, 2018, 8:43 a.m. | #1
Hi Angelo,

On 08/06/2018 12:12, Angelo Compagnucci wrote:
> Encryption on ubi volumes is actually broken cause a mismatch between
> the size expected to be written and the actual size after decrypting.
> Actually, the size to be written on disk is retrieved from the size of
> the encrypted image when the real size of a decrypted image is actually
> smaller.
> 
> This patch adds a "decrypted-size" attribute to the sw-description to
> explicitly tell to swupdate to allocate that size for ubi volumes.
> This parameter should be updated to the real size of the image just
> before assembling the update.
> 
> Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>


The "decrypted-size" is something we need just in case of UBI volumes. I
tend to put it as handler specific attribute instead of adding as
general attribute.

There is two ways for handler specific attributes. The generic attribute
"data" is simply parsed and not evaluated. and can be used for it.
Anyway, I have added "properties" that are solved as a list to each
image and passed to the handler. Both cases can be accepted, that is :

	data = "decrypted-size:value"

or (preferrable way):

properties: {
	decrypted-size =  "value";
}

That means in your case something like:

images: (
 {
  filename = "zImage.enc";
  volume = "kernel";
  type = "ubivol";
  encrypted = true;
  properties: {
	decrypted-size =  "value";
  }

 },

> ---
>  handlers/ubivol_handler.c |  3 ++-
>  include/swupdate.h        |  1 +
>  parser/parse_external.c   | 16 ++++++++++++++++
>  parser/parser.c           | 15 +++++++++++++++
>  4 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/handlers/ubivol_handler.c b/handlers/ubivol_handler.c
> index 0c6fcbf..d41f98a 100644
> --- a/handlers/ubivol_handler.c
> +++ b/handlers/ubivol_handler.c
> @@ -45,6 +45,7 @@ static int update_volume(libubi_t libubi, struct img_type *img,
>  
>  	bytes = img->size;
>  	if (img->is_encrypted) {
> +		bytes = img->decrsize;
>  		if (img->compressed) {
>  			ERROR("Decryption of compressed UBI images not supported");
>  			return -1;
> @@ -53,7 +54,7 @@ static int update_volume(libubi_t libubi, struct img_type *img,
>  			ERROR("Encrypted image size (%lld) too small", bytes);
>  			return -1;
>  		}
> -		bytes -= AES_BLOCK_SIZE;
> +		TRACE("Image is crypted, decrypted size %d bytes", bytes);

Added Achille in CC - this solved his problem, I guess the openssl
command in his case was different. I would suggest you fallback to this
in case decrypted-size is not set.

>  	}
>  
>  	if (!libubi) {
> diff --git a/include/swupdate.h b/include/swupdate.h
> index 455e4ad..79987e2 100644
> --- a/include/swupdate.h
> +++ b/include/swupdate.h
> @@ -58,6 +58,7 @@ struct img_type {
>  	char extract_file[MAX_IMAGE_FNAME];
>  	char filesystem[MAX_IMAGE_FNAME];
>  	unsigned long long seek;
> +	unsigned long long decrsize;
>  	int required;
>  	int provided;
>  	int compressed;
> diff --git a/parser/parse_external.c b/parser/parse_external.c
> index 66ba3d5..5173f8f 100644
> --- a/parser/parse_external.c
> +++ b/parser/parse_external.c
> @@ -32,7 +32,9 @@ static void sw_append_stream(struct img_type *img, const char *key,
>  	       const char *value)
>  {
>  	const char offset[] = "offset";
> +	const char dsize[] = "decrypted-size";
>  	char seek_str[MAX_SEEK_STRING_SIZE];
> +	char decrsize_str[MAX_SEEK_STRING_SIZE];
>  	char *endp = NULL;
>  
>  	if (!strcmp(key, "type"))
> @@ -81,6 +83,20 @@ static void sw_append_stream(struct img_type *img, const char *key,
>  		} else
>  			img->seek = 0;
>  	}
> +	if (!strncmp(key, dsize, sizeof(dsize))) {
> +		strncpy(decrsize_str, value,
> +			sizeof(decrsize_str));
> +		if (strnlen(decrsize_str, MAX_SEEK_STRING_SIZE) != 0) {
> +			errno = 0;
> +			img->decrsize = ustrtoull(decrsize_str, &endp, 0);
> +			if (decrsize_str == endp || (img->decrsize == ULLONG_MAX && \
> +					errno == ERANGE)) {
> +				ERROR("decrypted-size argument: ustrtoull failed");
> +				return;
> +			}
> +		} else
> +			img->decrsize = 0;
> +	}

See above

>  	if (!strcmp(key, "script"))
>  		img->is_script = 1;
>  	if (!strcmp(key, "path"))
> diff --git a/parser/parser.c b/parser/parser.c
> index 7329810..48e4573 100644
> --- a/parser/parser.c
> +++ b/parser/parser.c
> @@ -231,6 +231,7 @@ static int run_embscript(parsertype p, void *elem, struct img_type *img,
>  static int parse_common_attributes(parsertype p, void *elem, struct img_type *image)
>  {
>  	char seek_str[MAX_SEEK_STRING_SIZE];
> +	char decrsize_str[MAX_SEEK_STRING_SIZE];
>  	char *endp = NULL;
>  
>  	/*
> @@ -238,6 +239,7 @@ static int parse_common_attributes(parsertype p, void *elem, struct img_type *im
>  	 * found, be sure that it is empty
>  	 */
>  	seek_str[0] = '\0';
> +	decrsize_str[0] = '\0';
>  
>  	GET_FIELD_STRING(p, elem, "name", image->id.name);
>  	GET_FIELD_STRING(p, elem, "version", image->id.version);
> @@ -249,6 +251,7 @@ static int parse_common_attributes(parsertype p, void *elem, struct img_type *im
>  	GET_FIELD_STRING(p, elem, "filesystem", image->filesystem);
>  	GET_FIELD_STRING(p, elem, "type", image->type);
>  	GET_FIELD_STRING(p, elem, "offset", seek_str);
> +	GET_FIELD_STRING(p, elem, "decrypted-size", decrsize_str);
>  	GET_FIELD_STRING(p, elem, "data", image->type_data);
>  	get_hash_value(p, elem, image->sha256);
>  
> @@ -264,6 +267,18 @@ static int parse_common_attributes(parsertype p, void *elem, struct img_type *im
>  	} else
>  		image->seek = 0;
>  
> +	/* convert the decrsize handling multiplicative suffixes */
> +	if (strnlen(decrsize_str, MAX_SEEK_STRING_SIZE) != 0) {
> +		errno = 0;
> +		image->decrsize = ustrtoull(decrsize_str, &endp, 0);
> +		if (decrsize_str == endp || (image->decrsize == ULLONG_MAX && \
> +			errno == ERANGE)) {
> +			ERROR("decrypted-size argument: ustrtoull failed");
> +			return -1;
> +		}
> +	} else
> +		image->decrsize = 0;
> +
>  	get_field(p, elem, "compressed", &image->compressed);
>  	get_field(p, elem, "installed-directly", &image->install_directly);
>  	get_field(p, elem, "preserve-attributes", &image->preserve_attributes);
> 

Best regards,
Stefano Babic

Patch

diff --git a/handlers/ubivol_handler.c b/handlers/ubivol_handler.c
index 0c6fcbf..d41f98a 100644
--- a/handlers/ubivol_handler.c
+++ b/handlers/ubivol_handler.c
@@ -45,6 +45,7 @@  static int update_volume(libubi_t libubi, struct img_type *img,
 
 	bytes = img->size;
 	if (img->is_encrypted) {
+		bytes = img->decrsize;
 		if (img->compressed) {
 			ERROR("Decryption of compressed UBI images not supported");
 			return -1;
@@ -53,7 +54,7 @@  static int update_volume(libubi_t libubi, struct img_type *img,
 			ERROR("Encrypted image size (%lld) too small", bytes);
 			return -1;
 		}
-		bytes -= AES_BLOCK_SIZE;
+		TRACE("Image is crypted, decrypted size %d bytes", bytes);
 	}
 
 	if (!libubi) {
diff --git a/include/swupdate.h b/include/swupdate.h
index 455e4ad..79987e2 100644
--- a/include/swupdate.h
+++ b/include/swupdate.h
@@ -58,6 +58,7 @@  struct img_type {
 	char extract_file[MAX_IMAGE_FNAME];
 	char filesystem[MAX_IMAGE_FNAME];
 	unsigned long long seek;
+	unsigned long long decrsize;
 	int required;
 	int provided;
 	int compressed;
diff --git a/parser/parse_external.c b/parser/parse_external.c
index 66ba3d5..5173f8f 100644
--- a/parser/parse_external.c
+++ b/parser/parse_external.c
@@ -32,7 +32,9 @@  static void sw_append_stream(struct img_type *img, const char *key,
 	       const char *value)
 {
 	const char offset[] = "offset";
+	const char dsize[] = "decrypted-size";
 	char seek_str[MAX_SEEK_STRING_SIZE];
+	char decrsize_str[MAX_SEEK_STRING_SIZE];
 	char *endp = NULL;
 
 	if (!strcmp(key, "type"))
@@ -81,6 +83,20 @@  static void sw_append_stream(struct img_type *img, const char *key,
 		} else
 			img->seek = 0;
 	}
+	if (!strncmp(key, dsize, sizeof(dsize))) {
+		strncpy(decrsize_str, value,
+			sizeof(decrsize_str));
+		if (strnlen(decrsize_str, MAX_SEEK_STRING_SIZE) != 0) {
+			errno = 0;
+			img->decrsize = ustrtoull(decrsize_str, &endp, 0);
+			if (decrsize_str == endp || (img->decrsize == ULLONG_MAX && \
+					errno == ERANGE)) {
+				ERROR("decrypted-size argument: ustrtoull failed");
+				return;
+			}
+		} else
+			img->decrsize = 0;
+	}
 	if (!strcmp(key, "script"))
 		img->is_script = 1;
 	if (!strcmp(key, "path"))
diff --git a/parser/parser.c b/parser/parser.c
index 7329810..48e4573 100644
--- a/parser/parser.c
+++ b/parser/parser.c
@@ -231,6 +231,7 @@  static int run_embscript(parsertype p, void *elem, struct img_type *img,
 static int parse_common_attributes(parsertype p, void *elem, struct img_type *image)
 {
 	char seek_str[MAX_SEEK_STRING_SIZE];
+	char decrsize_str[MAX_SEEK_STRING_SIZE];
 	char *endp = NULL;
 
 	/*
@@ -238,6 +239,7 @@  static int parse_common_attributes(parsertype p, void *elem, struct img_type *im
 	 * found, be sure that it is empty
 	 */
 	seek_str[0] = '\0';
+	decrsize_str[0] = '\0';
 
 	GET_FIELD_STRING(p, elem, "name", image->id.name);
 	GET_FIELD_STRING(p, elem, "version", image->id.version);
@@ -249,6 +251,7 @@  static int parse_common_attributes(parsertype p, void *elem, struct img_type *im
 	GET_FIELD_STRING(p, elem, "filesystem", image->filesystem);
 	GET_FIELD_STRING(p, elem, "type", image->type);
 	GET_FIELD_STRING(p, elem, "offset", seek_str);
+	GET_FIELD_STRING(p, elem, "decrypted-size", decrsize_str);
 	GET_FIELD_STRING(p, elem, "data", image->type_data);
 	get_hash_value(p, elem, image->sha256);
 
@@ -264,6 +267,18 @@  static int parse_common_attributes(parsertype p, void *elem, struct img_type *im
 	} else
 		image->seek = 0;
 
+	/* convert the decrsize handling multiplicative suffixes */
+	if (strnlen(decrsize_str, MAX_SEEK_STRING_SIZE) != 0) {
+		errno = 0;
+		image->decrsize = ustrtoull(decrsize_str, &endp, 0);
+		if (decrsize_str == endp || (image->decrsize == ULLONG_MAX && \
+			errno == ERANGE)) {
+			ERROR("decrypted-size argument: ustrtoull failed");
+			return -1;
+		}
+	} else
+		image->decrsize = 0;
+
 	get_field(p, elem, "compressed", &image->compressed);
 	get_field(p, elem, "installed-directly", &image->install_directly);
 	get_field(p, elem, "preserve-attributes", &image->preserve_attributes);