mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH] commands/digest: add verify support
@ 2014-09-24 13:21 Eric Bénard
  2014-09-25  7:15 ` Uwe Kleine-König
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Bénard @ 2014-09-24 13:21 UTC (permalink / raw)
  To: barebox

From: Hubert Feurstein <h.feurstein@gmail.com>

Signed-off-by: Hubert Feurstein <h.feurstein@gmail.com>
[EB]: reworked based on Sascha's comments and tested with md5sum
Signed-off-by: Eric Bénard <eric@eukrea.com>
---
 commands/digest.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 110 insertions(+), 10 deletions(-)

diff --git a/commands/digest.c b/commands/digest.c
index 092fda2..2a982e5 100644
--- a/commands/digest.c
+++ b/commands/digest.c
@@ -23,26 +23,99 @@
 #include <fcntl.h>
 #include <errno.h>
 #include <xfuncs.h>
+#include <libfile.h>
 #include <malloc.h>
 #include <digest.h>
+#include <linux/ctype.h>
+#include <getopt.h>
+
+static inline unsigned char parse_hexchar(char c)
+{
+	if (!isxdigit(c))
+		return 0;
+
+	return isdigit(c) ? (c - '0') : ((islower(c) ? toupper(c) : c) - 'A' + 0xA);
+}
+
+static inline unsigned char parse_hexbyte(const char *p)
+{
+	return (parse_hexchar(*p) << 4) | parse_hexchar(*(p + 1));
+}
+
+static unsigned char *parse_hash(int hash_len, const char *hexstr)
+{
+	int i;
+	unsigned char *p;
+
+	p = calloc(hash_len, sizeof(unsigned char));
+	if (!p) {
+		perror("calloc");
+		return NULL;
+	}
+
+	for (i = 0; i < hash_len; i++)
+		p[i] = parse_hexbyte(&hexstr[i * 2]);
+
+	return p;
+}
 
 static int do_digest(char *algorithm, int argc, char *argv[])
 {
 	struct digest *d;
 	int ret = 0;
 	int i;
-	unsigned char *hash;
+	unsigned char *hash = NULL;
+	int opt;
+	unsigned char *verify_hash = NULL;
+	int verify = 0;
+	int min_argc = 2;
+	int len = 0;
+	ssize_t bufsz;
 
 	d = digest_get_by_name(algorithm);
 	BUG_ON(!d);
 
-	if (argc < 2)
-		return COMMAND_ERROR_USAGE;
+	while ((opt = getopt(argc, argv, "v:V:")) > 0) {
+		switch (opt) {
+		case 'v':
+			verify++;
+			free(verify_hash);
+			hash = xstrdup(optarg);
+			len = strlen(hash);
+			break;
+		case 'V':
+			verify++;
+			free(verify_hash);
+			hash = read_file(optarg, &bufsz);
+			len = bufsz;
+			break;
+		default:
+			ret = COMMAND_ERROR_USAGE;
+			goto out;
+		}
+
+		if (verify == 1 && (d->length * 2) <= len && hash != NULL) {
+			min_argc += 2;
+			verify_hash = parse_hash(d->length, hash);
+			if (!verify_hash)
+				return -ENOMEM;
+			free(hash);
+		} else
+			return COMMAND_ERROR_USAGE;
+	}
+
+	if (argc < min_argc) {
+		ret = COMMAND_ERROR_USAGE;
+		goto out;
+	}
+
+	argv += min_argc - 2;
 
 	hash = calloc(d->length, sizeof(unsigned char));
 	if (!hash) {
 		perror("calloc");
-		return COMMAND_ERROR_USAGE;
+		ret = -ENOMEM;
+		goto out;
 	}
 
 	argv++;
@@ -60,17 +133,32 @@ static int do_digest(char *algorithm, int argc, char *argv[])
 		if (digest_file_window(d, filename, hash, start, size) < 0) {
 			ret = 1;
 		} else {
-			for (i = 0; i < d->length; i++)
+			for (i = 0; i < d->length; i++) {
 				printf("%02x", hash[i]);
+				if (verify > 0 && hash[i] != verify_hash[i])
+					verify = -1;
+			}
 
-			printf("  %s\t0x%08llx ... 0x%08llx\n",
+			printf("  %s\t0x%08llx ... 0x%08llx",
 				filename, start, start + size);
+
+			if (verify < 0) {
+				printf(" ** ERROR ** ");
+				ret = 1;
+			}
+
+			printf("\n");
+
+			if (verify)
+				break;
 		}
 
 		argv++;
 	}
 
+out:
 	free(hash);
+	free(verify_hash);
 
 	return ret;
 }
@@ -84,12 +172,15 @@ static int do_md5(int argc, char *argv[])
 
 BAREBOX_CMD_HELP_START(md5sum)
 BAREBOX_CMD_HELP_TEXT("Calculate a MD5 digest over a FILE or a memory area.")
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT ("-v hash",     "verify")
+BAREBOX_CMD_HELP_OPT ("-V hash-file", "verify hash file")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(md5sum)
 	.cmd		= do_md5,
 	BAREBOX_CMD_DESC("calculate MD5 checksum")
-	BAREBOX_CMD_OPTS("FILE|AREA...")
+	BAREBOX_CMD_OPTS("[-vV] FILE|AREA...")
 	BAREBOX_CMD_GROUP(CMD_GRP_FILE)
 	BAREBOX_CMD_HELP(cmd_md5sum_help)
 BAREBOX_CMD_END
@@ -105,12 +196,15 @@ static int do_sha1(int argc, char *argv[])
 
 BAREBOX_CMD_HELP_START(sha1sum)
 BAREBOX_CMD_HELP_TEXT("Calculate a SHA1 digest over a FILE or a memory area.")
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT ("-v hash",     "verify")
+BAREBOX_CMD_HELP_OPT ("-V hash-file", "verify hash file")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(sha1sum)
 	.cmd		= do_sha1,
 	BAREBOX_CMD_DESC("calculate SHA1 digest")
-	BAREBOX_CMD_OPTS("FILE|AREA")
+	BAREBOX_CMD_OPTS("[-vV] FILE|AREA")
 	BAREBOX_CMD_GROUP(CMD_GRP_FILE)
 	BAREBOX_CMD_HELP(cmd_sha1sum_help)
 BAREBOX_CMD_END
@@ -126,12 +220,15 @@ static int do_sha224(int argc, char *argv[])
 
 BAREBOX_CMD_HELP_START(sha224sum)
 BAREBOX_CMD_HELP_TEXT("Calculate a SHA224 digest over a FILE or a memory area.")
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT ("-v hash",     "verify")
+BAREBOX_CMD_HELP_OPT ("-V hash-file", "verify hash file")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(sha224sum)
 	.cmd		= do_sha224,
 	BAREBOX_CMD_DESC("calculate SHA224 digest")
-	BAREBOX_CMD_OPTS("FILE|AREA")
+	BAREBOX_CMD_OPTS("[-vV] FILE|AREA")
 	BAREBOX_CMD_GROUP(CMD_GRP_FILE)
 	BAREBOX_CMD_HELP(cmd_sha224sum_help)
 BAREBOX_CMD_END
@@ -147,12 +244,15 @@ static int do_sha256(int argc, char *argv[])
 
 BAREBOX_CMD_HELP_START(sha256sum)
 BAREBOX_CMD_HELP_TEXT("Calculate a SHA256 digest over a FILE or a memory area.")
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT ("-v hash",     "verify")
+BAREBOX_CMD_HELP_OPT ("-V hash-file", "verify hash file")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(sha256sum)
 	.cmd		= do_sha256,
 	BAREBOX_CMD_DESC("calculate SHA256 digest")
-	BAREBOX_CMD_OPTS("FILE|AREA")
+	BAREBOX_CMD_OPTS("[-vV] FILE|AREA")
 	BAREBOX_CMD_GROUP(CMD_GRP_FILE)
 	BAREBOX_CMD_HELP(cmd_sha256sum_help)
 BAREBOX_CMD_END
-- 
1.9.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] commands/digest: add verify support
  2014-09-24 13:21 [PATCH] commands/digest: add verify support Eric Bénard
@ 2014-09-25  7:15 ` Uwe Kleine-König
  0 siblings, 0 replies; 6+ messages in thread
From: Uwe Kleine-König @ 2014-09-25  7:15 UTC (permalink / raw)
  To: Eric Bénard; +Cc: barebox

Hello Eric,

On Wed, Sep 24, 2014 at 03:21:10PM +0200, Eric Bénard wrote:
> From: Hubert Feurstein <h.feurstein@gmail.com>
> 
> Signed-off-by: Hubert Feurstein <h.feurstein@gmail.com>
> [EB]: reworked based on Sascha's comments and tested with md5sum
> Signed-off-by: Eric Bénard <eric@eukrea.com>
> ---
>  commands/digest.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 110 insertions(+), 10 deletions(-)
> 
> diff --git a/commands/digest.c b/commands/digest.c
> index 092fda2..2a982e5 100644
> --- a/commands/digest.c
> +++ b/commands/digest.c
> @@ -23,26 +23,99 @@
>  #include <fcntl.h>
>  #include <errno.h>
>  #include <xfuncs.h>
> +#include <libfile.h>
>  #include <malloc.h>
>  #include <digest.h>
> +#include <linux/ctype.h>
> +#include <getopt.h>
> +
> +static inline unsigned char parse_hexchar(char c)
> +{
> +	if (!isxdigit(c))
> +		return 0;
> +
> +	return isdigit(c) ? (c - '0') : ((islower(c) ? toupper(c) : c) - 'A' + 0xA);
Probably this is fine in barebox and also the check for isxdigit above
should make it save, but in general this approach might result in
surprises. In general isdigit et al are 1) locale dependant and 2) all
take an unsigned char (or EOF).

(Using Python here, but the same applies to C code obviously.)

With LC_ALL=de_DE LANG=de_DE the first fact yields:

$ python
Python 2.7.7 (default, Jun  3 2014, 16:16:56) 
[GCC 4.8.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import locale
>>> locale.setlocale(locale.LC_ALL, '')
'de_DE'
>>> print 'Ä'.isupper()
True

The problem with unsigned is that

	ispunct('\xfc')

might result in checking -4 which is undefined.

This obviously only applies to platforms where char is implicitly
signed (According to https://wiki.debian.org/ArchitectureSpecificsMemo
this includes x86 and mips.)

So I'd suggest to check the ASCII-values explicitly, something like:

	switch (c) {
	case '0'..'9':
		return c - '0';
		break;
	case 'a'..'f':
		return c - 'a' + 0xa;
		break;
	case 'A'..'F':
		return c - 'A' + 0xa;
		break;
	default:
		return 0;
		break;
	}

to be on the safe side and to also be able to use the code in userspace.
(But note this is even safe EBCDIC machines, but only because we're
using only the letters up to 'f'. 'j' - 'a' is 16 there. :-)

Best regards
Uwe, the pedantic

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] commands/digest: add verify support
  2015-02-16 13:02 h.feurstein
@ 2015-02-17  7:39 ` Sascha Hauer
  0 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2015-02-17  7:39 UTC (permalink / raw)
  To: h.feurstein; +Cc: barebox

Hi Hubert,

On Mon, Feb 16, 2015 at 02:02:35PM +0100, h.feurstein@gmail.com wrote:
> From: Hubert Feurstein <h.feurstein@gmail.com>
> 
> Signed-off-by: Hubert Feurstein <h.feurstein@gmail.com>
> [EB]: reworked based on Sascha's comments and tested with md5sum
> Signed-off-by: Eric Bénard <eric@eukrea.com>
> ---
> -	if (argc < 2)
> -		return COMMAND_ERROR_USAGE;
> +	while ((opt = getopt(argc, argv, "v:V:")) > 0) {
> +		switch (opt) {
> +		case 'v':
> +			verify++;
> +			free(verify_hash);
> +			hash = xstrdup(optarg);
> +			len = strlen(hash);
> +			break;
> +		case 'V':
> +			verify++;
> +			free(verify_hash);
> +			hash = read_file(optarg, &bufsz);
> +			len = bufsz;
> +			break;
> +		default:
> +			ret = COMMAND_ERROR_USAGE;
> +			goto out;
> +		}
> +
> +		if (verify == 1 && (d->length * 2) <= len && hash != NULL) {
> +			min_argc += 2;
> +			verify_hash = parse_hash(d->length, hash);
> +			if (!verify_hash)
> +				return -ENOMEM;
> +			free(hash);
> +		} else
> +			return COMMAND_ERROR_USAGE;

This code in the option parsing loop looks odd. Could you just parse
options in the loop, store the hash(file) in a variable and do the
processing outside the loop?

> +	}
> +
> +	if (argc < min_argc) {
> +		ret = COMMAND_ERROR_USAGE;
> +		goto out;
> +	}
> +
> +	argv += min_argc - 2;
>  
>  	hash = calloc(d->length, sizeof(unsigned char));

xzalloc should be safe here.

>  	if (!hash) {
>  		perror("calloc");
> -		return COMMAND_ERROR_USAGE;
> +		ret = -ENOMEM;
> +		goto out;
>  	}
>  
>  	argv++;
> @@ -60,17 +133,32 @@ static int do_digest(char *algorithm, int argc, char *argv[])
>  		if (digest_file_window(d, filename, hash, start, size) < 0) {
>  			ret = 1;
>  		} else {
> -			for (i = 0; i < d->length; i++)
> +			for (i = 0; i < d->length; i++) {
>  				printf("%02x", hash[i]);
> +				if (verify > 0 && hash[i] != verify_hash[i])
> +					verify = -1;
> +			}
>  
> -			printf("  %s\t0x%08llx ... 0x%08llx\n",
> +			printf("  %s\t0x%08llx ... 0x%08llx",
>  				filename, start, start + size);
> +
> +			if (verify < 0) {
> +				printf(" ** ERROR ** ");

Printing the real hash and the expected hash would be nice here.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] commands/digest: add verify support
@ 2015-02-16 13:02 h.feurstein
  2015-02-17  7:39 ` Sascha Hauer
  0 siblings, 1 reply; 6+ messages in thread
From: h.feurstein @ 2015-02-16 13:02 UTC (permalink / raw)
  To: barebox

From: Hubert Feurstein <h.feurstein@gmail.com>

Signed-off-by: Hubert Feurstein <h.feurstein@gmail.com>
[EB]: reworked based on Sascha's comments and tested with md5sum
Signed-off-by: Eric Bénard <eric@eukrea.com>
---
 commands/digest.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 110 insertions(+), 10 deletions(-)

diff --git a/commands/digest.c b/commands/digest.c
index 092fda2..2a982e5 100644
--- a/commands/digest.c
+++ b/commands/digest.c
@@ -23,26 +23,99 @@
 #include <fcntl.h>
 #include <errno.h>
 #include <xfuncs.h>
+#include <libfile.h>
 #include <malloc.h>
 #include <digest.h>
+#include <linux/ctype.h>
+#include <getopt.h>
+
+static inline unsigned char parse_hexchar(char c)
+{
+	if (!isxdigit(c))
+		return 0;
+
+	return isdigit(c) ? (c - '0') : ((islower(c) ? toupper(c) : c) - 'A' + 0xA);
+}
+
+static inline unsigned char parse_hexbyte(const char *p)
+{
+	return (parse_hexchar(*p) << 4) | parse_hexchar(*(p + 1));
+}
+
+static unsigned char *parse_hash(int hash_len, const char *hexstr)
+{
+	int i;
+	unsigned char *p;
+
+	p = calloc(hash_len, sizeof(unsigned char));
+	if (!p) {
+		perror("calloc");
+		return NULL;
+	}
+
+	for (i = 0; i < hash_len; i++)
+		p[i] = parse_hexbyte(&hexstr[i * 2]);
+
+	return p;
+}
 
 static int do_digest(char *algorithm, int argc, char *argv[])
 {
 	struct digest *d;
 	int ret = 0;
 	int i;
-	unsigned char *hash;
+	unsigned char *hash = NULL;
+	int opt;
+	unsigned char *verify_hash = NULL;
+	int verify = 0;
+	int min_argc = 2;
+	int len = 0;
+	ssize_t bufsz;
 
 	d = digest_get_by_name(algorithm);
 	BUG_ON(!d);
 
-	if (argc < 2)
-		return COMMAND_ERROR_USAGE;
+	while ((opt = getopt(argc, argv, "v:V:")) > 0) {
+		switch (opt) {
+		case 'v':
+			verify++;
+			free(verify_hash);
+			hash = xstrdup(optarg);
+			len = strlen(hash);
+			break;
+		case 'V':
+			verify++;
+			free(verify_hash);
+			hash = read_file(optarg, &bufsz);
+			len = bufsz;
+			break;
+		default:
+			ret = COMMAND_ERROR_USAGE;
+			goto out;
+		}
+
+		if (verify == 1 && (d->length * 2) <= len && hash != NULL) {
+			min_argc += 2;
+			verify_hash = parse_hash(d->length, hash);
+			if (!verify_hash)
+				return -ENOMEM;
+			free(hash);
+		} else
+			return COMMAND_ERROR_USAGE;
+	}
+
+	if (argc < min_argc) {
+		ret = COMMAND_ERROR_USAGE;
+		goto out;
+	}
+
+	argv += min_argc - 2;
 
 	hash = calloc(d->length, sizeof(unsigned char));
 	if (!hash) {
 		perror("calloc");
-		return COMMAND_ERROR_USAGE;
+		ret = -ENOMEM;
+		goto out;
 	}
 
 	argv++;
@@ -60,17 +133,32 @@ static int do_digest(char *algorithm, int argc, char *argv[])
 		if (digest_file_window(d, filename, hash, start, size) < 0) {
 			ret = 1;
 		} else {
-			for (i = 0; i < d->length; i++)
+			for (i = 0; i < d->length; i++) {
 				printf("%02x", hash[i]);
+				if (verify > 0 && hash[i] != verify_hash[i])
+					verify = -1;
+			}
 
-			printf("  %s\t0x%08llx ... 0x%08llx\n",
+			printf("  %s\t0x%08llx ... 0x%08llx",
 				filename, start, start + size);
+
+			if (verify < 0) {
+				printf(" ** ERROR ** ");
+				ret = 1;
+			}
+
+			printf("\n");
+
+			if (verify)
+				break;
 		}
 
 		argv++;
 	}
 
+out:
 	free(hash);
+	free(verify_hash);
 
 	return ret;
 }
@@ -84,12 +172,15 @@ static int do_md5(int argc, char *argv[])
 
 BAREBOX_CMD_HELP_START(md5sum)
 BAREBOX_CMD_HELP_TEXT("Calculate a MD5 digest over a FILE or a memory area.")
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT ("-v hash",     "verify")
+BAREBOX_CMD_HELP_OPT ("-V hash-file", "verify hash file")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(md5sum)
 	.cmd		= do_md5,
 	BAREBOX_CMD_DESC("calculate MD5 checksum")
-	BAREBOX_CMD_OPTS("FILE|AREA...")
+	BAREBOX_CMD_OPTS("[-vV] FILE|AREA...")
 	BAREBOX_CMD_GROUP(CMD_GRP_FILE)
 	BAREBOX_CMD_HELP(cmd_md5sum_help)
 BAREBOX_CMD_END
@@ -105,12 +196,15 @@ static int do_sha1(int argc, char *argv[])
 
 BAREBOX_CMD_HELP_START(sha1sum)
 BAREBOX_CMD_HELP_TEXT("Calculate a SHA1 digest over a FILE or a memory area.")
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT ("-v hash",     "verify")
+BAREBOX_CMD_HELP_OPT ("-V hash-file", "verify hash file")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(sha1sum)
 	.cmd		= do_sha1,
 	BAREBOX_CMD_DESC("calculate SHA1 digest")
-	BAREBOX_CMD_OPTS("FILE|AREA")
+	BAREBOX_CMD_OPTS("[-vV] FILE|AREA")
 	BAREBOX_CMD_GROUP(CMD_GRP_FILE)
 	BAREBOX_CMD_HELP(cmd_sha1sum_help)
 BAREBOX_CMD_END
@@ -126,12 +220,15 @@ static int do_sha224(int argc, char *argv[])
 
 BAREBOX_CMD_HELP_START(sha224sum)
 BAREBOX_CMD_HELP_TEXT("Calculate a SHA224 digest over a FILE or a memory area.")
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT ("-v hash",     "verify")
+BAREBOX_CMD_HELP_OPT ("-V hash-file", "verify hash file")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(sha224sum)
 	.cmd		= do_sha224,
 	BAREBOX_CMD_DESC("calculate SHA224 digest")
-	BAREBOX_CMD_OPTS("FILE|AREA")
+	BAREBOX_CMD_OPTS("[-vV] FILE|AREA")
 	BAREBOX_CMD_GROUP(CMD_GRP_FILE)
 	BAREBOX_CMD_HELP(cmd_sha224sum_help)
 BAREBOX_CMD_END
@@ -147,12 +244,15 @@ static int do_sha256(int argc, char *argv[])
 
 BAREBOX_CMD_HELP_START(sha256sum)
 BAREBOX_CMD_HELP_TEXT("Calculate a SHA256 digest over a FILE or a memory area.")
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT ("-v hash",     "verify")
+BAREBOX_CMD_HELP_OPT ("-V hash-file", "verify hash file")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(sha256sum)
 	.cmd		= do_sha256,
 	BAREBOX_CMD_DESC("calculate SHA256 digest")
-	BAREBOX_CMD_OPTS("FILE|AREA")
+	BAREBOX_CMD_OPTS("[-vV] FILE|AREA")
 	BAREBOX_CMD_GROUP(CMD_GRP_FILE)
 	BAREBOX_CMD_HELP(cmd_sha256sum_help)
 BAREBOX_CMD_END
-- 
2.3.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] commands/digest: add verify support
  2013-04-19  8:43 Hubert Feurstein
@ 2013-04-21 11:22 ` Sascha Hauer
  0 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2013-04-21 11:22 UTC (permalink / raw)
  To: Hubert Feurstein; +Cc: barebox

On Fri, Apr 19, 2013 at 10:43:55AM +0200, Hubert Feurstein wrote:
> Signed-off-by: Hubert Feurstein <h.feurstein@gmail.com>
> ---
>  commands/digest.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 83 insertions(+), 11 deletions(-)
> 
> diff --git a/commands/digest.c b/commands/digest.c
> index c9bb132..56173a5 100644
> --- a/commands/digest.c
> +++ b/commands/digest.c
> @@ -25,24 +25,76 @@
>  #include <xfuncs.h>
>  #include <malloc.h>
>  #include <digest.h>
> +#include <linux/ctype.h>
> +#include <getopt.h>
> +
> +static inline unsigned char parse_hexchar(char c)
> +{
> +	if (!isxdigit(c))
> +		return 0;
> +
> +	return isdigit(c) ? (c - '0') : ((islower(c) ? toupper(c) : c) - 'A' + 0xA);
> +}
> +
> +static inline unsigned char parse_hexbyte(const char *p)
> +{
> +	return (parse_hexchar(*p) << 4) | parse_hexchar(*(p + 1));
> +}
>  
>  static int do_digest(char *algorithm, int argc, char *argv[])
>  {
>  	struct digest *d;
>  	int ret = 0;
>  	int i;
> -	unsigned char *hash;
> +	unsigned char *hash = 0;
> +	int opt;
> +	unsigned char *verify_hash = 0;

NULL for pointers please.

> +	int verify = 0;
> +	int min_argc = 2;
>  
>  	d = digest_get_by_name(algorithm);
>  	BUG_ON(!d);
>  
> -	if (argc < 2)
> -		return COMMAND_ERROR_USAGE;
> +	while ((opt = getopt(argc, argv, "v:")) > 0) {
> +		switch (opt) {
> +		case 'v':
> +			verify = 1;
> +			min_argc += 2;
> +
> +			if (d->length != (strlen(optarg) / 2)) {
> +				ret = COMMAND_ERROR_USAGE;
> +				goto out;
> +			}

Maybe give the user a hint here? It's not really obvious to the user
what's wrong here.

> +
> +			verify_hash = calloc(d->length, sizeof(unsigned char));
> +			if (!verify_hash) {
> +				perror("calloc");
> +				ret = -ENOMEM;
> +				goto out;
> +			}
> +
> +			for (i = 0; i < d->length; i++)
> +				verify_hash[i] = parse_hexbyte(&optarg[i * 2]);
> +
> +			break;

You should only save optarg for later use in the loop. Otherwise when
somebody gives -v multiple times you allocate memory each time which you
don't free later.

> +		default:
> +			ret = COMMAND_ERROR_USAGE;
> +			goto out;
> +		}
> +	}
> +
> +	if (argc < min_argc) {
> +		ret = COMMAND_ERROR_USAGE;
> +		goto out;
> +	}
> +
> +	argv += min_argc - 2;
>  
>  	hash = calloc(d->length, sizeof(unsigned char));
>  	if (!hash) {
>  		perror("calloc");
> -		return COMMAND_ERROR_USAGE;
> +		ret = -ENOMEM;
> +		goto out;
>  	}
>  
>  	argv++;
> @@ -60,17 +112,33 @@ static int do_digest(char *algorithm, int argc, char *argv[])
>  		if (digest_file_window(d, filename, hash, start, size) < 0) {
>  			ret = 1;
>  		} else {
> -			for (i = 0; i < d->length; i++)
> +			for (i = 0; i < d->length; i++) {
>  				printf("%02x", hash[i]);
> +				if (verify > 0 && hash[i] != verify_hash[i])
> +					verify = -1;
> +			}
>  
> -			printf("  %s\t0x%08llx ... 0x%08llx\n",
> +			printf("  %s\t0x%08llx ... 0x%08llx",
>  				filename, start, start + size);
> +
> +			if (verify < 0) {
> +				printf(" ** ERROR **");
> +				ret = 1;
> +				verify = 1;
> +			}
> +
> +			printf("\n");
>  		}

The digest commands can work on multiple files. When verifying you
verify multiple files against a single hash. Is this really useful
behaviour? I'd suggest to allow only a single file in verify mode.

>  
>  		argv++;
>  	}
>  
> -	free(hash);
> +out:
> +	if (hash)
> +		free(hash);
> +
> +	if (verify_hash)
> +		free(verify_hash);

No need to check. free() works fine on NULL pointers.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] commands/digest: add verify support
@ 2013-04-19  8:43 Hubert Feurstein
  2013-04-21 11:22 ` Sascha Hauer
  0 siblings, 1 reply; 6+ messages in thread
From: Hubert Feurstein @ 2013-04-19  8:43 UTC (permalink / raw)
  To: barebox

Signed-off-by: Hubert Feurstein <h.feurstein@gmail.com>
---
 commands/digest.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 83 insertions(+), 11 deletions(-)

diff --git a/commands/digest.c b/commands/digest.c
index c9bb132..56173a5 100644
--- a/commands/digest.c
+++ b/commands/digest.c
@@ -25,24 +25,76 @@
 #include <xfuncs.h>
 #include <malloc.h>
 #include <digest.h>
+#include <linux/ctype.h>
+#include <getopt.h>
+
+static inline unsigned char parse_hexchar(char c)
+{
+	if (!isxdigit(c))
+		return 0;
+
+	return isdigit(c) ? (c - '0') : ((islower(c) ? toupper(c) : c) - 'A' + 0xA);
+}
+
+static inline unsigned char parse_hexbyte(const char *p)
+{
+	return (parse_hexchar(*p) << 4) | parse_hexchar(*(p + 1));
+}
 
 static int do_digest(char *algorithm, int argc, char *argv[])
 {
 	struct digest *d;
 	int ret = 0;
 	int i;
-	unsigned char *hash;
+	unsigned char *hash = 0;
+	int opt;
+	unsigned char *verify_hash = 0;
+	int verify = 0;
+	int min_argc = 2;
 
 	d = digest_get_by_name(algorithm);
 	BUG_ON(!d);
 
-	if (argc < 2)
-		return COMMAND_ERROR_USAGE;
+	while ((opt = getopt(argc, argv, "v:")) > 0) {
+		switch (opt) {
+		case 'v':
+			verify = 1;
+			min_argc += 2;
+
+			if (d->length != (strlen(optarg) / 2)) {
+				ret = COMMAND_ERROR_USAGE;
+				goto out;
+			}
+
+			verify_hash = calloc(d->length, sizeof(unsigned char));
+			if (!verify_hash) {
+				perror("calloc");
+				ret = -ENOMEM;
+				goto out;
+			}
+
+			for (i = 0; i < d->length; i++)
+				verify_hash[i] = parse_hexbyte(&optarg[i * 2]);
+
+			break;
+		default:
+			ret = COMMAND_ERROR_USAGE;
+			goto out;
+		}
+	}
+
+	if (argc < min_argc) {
+		ret = COMMAND_ERROR_USAGE;
+		goto out;
+	}
+
+	argv += min_argc - 2;
 
 	hash = calloc(d->length, sizeof(unsigned char));
 	if (!hash) {
 		perror("calloc");
-		return COMMAND_ERROR_USAGE;
+		ret = -ENOMEM;
+		goto out;
 	}
 
 	argv++;
@@ -60,17 +112,33 @@ static int do_digest(char *algorithm, int argc, char *argv[])
 		if (digest_file_window(d, filename, hash, start, size) < 0) {
 			ret = 1;
 		} else {
-			for (i = 0; i < d->length; i++)
+			for (i = 0; i < d->length; i++) {
 				printf("%02x", hash[i]);
+				if (verify > 0 && hash[i] != verify_hash[i])
+					verify = -1;
+			}
 
-			printf("  %s\t0x%08llx ... 0x%08llx\n",
+			printf("  %s\t0x%08llx ... 0x%08llx",
 				filename, start, start + size);
+
+			if (verify < 0) {
+				printf(" ** ERROR **");
+				ret = 1;
+				verify = 1;
+			}
+
+			printf("\n");
 		}
 
 		argv++;
 	}
 
-	free(hash);
+out:
+	if (hash)
+		free(hash);
+
+	if (verify_hash)
+		free(verify_hash);
 
 	return ret;
 }
@@ -83,8 +151,9 @@ static int do_md5(int argc, char *argv[])
 }
 
 BAREBOX_CMD_HELP_START(md5sum)
-BAREBOX_CMD_HELP_USAGE("md5sum [[FILE] [AREA]]...\n")
+BAREBOX_CMD_HELP_USAGE("md5sum [OPTION] [[FILE] [AREA]]...\n")
 BAREBOX_CMD_HELP_SHORT("Calculate a md5 checksum of a memory area.\n")
+BAREBOX_CMD_HELP_OPT  ("-v <hash>",  "Verify\n")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(md5sum)
@@ -103,8 +172,9 @@ static int do_sha1(int argc, char *argv[])
 }
 
 BAREBOX_CMD_HELP_START(sha1sum)
-BAREBOX_CMD_HELP_USAGE("sha1sum [[FILE] [AREA]]...\n")
+BAREBOX_CMD_HELP_USAGE("sha1sum [OPTION] [[FILE] [AREA]]...\n")
 BAREBOX_CMD_HELP_SHORT("Calculate a sha1 checksum of a memory area.\n")
+BAREBOX_CMD_HELP_OPT  ("-v <hash>",  "Verify\n")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(sha1sum)
@@ -123,8 +193,9 @@ static int do_sha224(int argc, char *argv[])
 }
 
 BAREBOX_CMD_HELP_START(sha224sum)
-BAREBOX_CMD_HELP_USAGE("sha224sum [[FILE] [AREA]]...\n")
+BAREBOX_CMD_HELP_USAGE("sha224sum [OPTION] [[FILE] [AREA]]...\n")
 BAREBOX_CMD_HELP_SHORT("Calculate a sha224 checksum of a memory area.\n")
+BAREBOX_CMD_HELP_OPT  ("-v <hash>",  "Verify\n")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(sha224sum)
@@ -143,8 +214,9 @@ static int do_sha256(int argc, char *argv[])
 }
 
 BAREBOX_CMD_HELP_START(sha256sum)
-BAREBOX_CMD_HELP_USAGE("sha256sum [[FILE] [AREA]]...\n")
+BAREBOX_CMD_HELP_USAGE("sha256sum [OPTION] [[FILE] [AREA]]...\n")
 BAREBOX_CMD_HELP_SHORT("Calculate a sha256 checksum of a memory area.\n")
+BAREBOX_CMD_HELP_OPT  ("-v <hash>",  "Verify\n")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(sha256sum)
-- 
1.8.1.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-02-17  7:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-24 13:21 [PATCH] commands/digest: add verify support Eric Bénard
2014-09-25  7:15 ` Uwe Kleine-König
  -- strict thread matches above, loose matches on Subject: below --
2015-02-16 13:02 h.feurstein
2015-02-17  7:39 ` Sascha Hauer
2013-04-19  8:43 Hubert Feurstein
2013-04-21 11:22 ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox