On Tue, Dec 25, 2018 at 12:01:12PM +0100, grub-de...@agowa338.de wrote:
> Hello,
>
> I want to use grub-mkpasswd inside of a ansible playbook, therefore a
> idempotency or check feature is required. Without it, there is no
> posibility to verify, that the configuration is already inplace.
> Therefore I want to suggest implementing the attached changes. The
> simplest one I was thinking of, is grub-mkpasswd allowing to specify a
> salt and comparing the results.
I am not sure I understand. Could you elaborate on this?
> My patch also introduces a quiet mode, that basically suppresses all
> prompts and only expects the password to be entered/piped in once. It
> also changes the output slightly to not inlcude the `PBKDF2 hash of
> your password is `-part.
It seems to me that it should be split into 3 parts...
> The patch is attached to this mail.
>
> Best,
> Klaus Frank
> >From d289bbb5f5dffbbed2c871989f55e48b756e9ae2 Mon Sep 17 00:00:00 2001
> From: Klaus Frank <g...@frank.fyi>
> Date: Tue, 25 Dec 2018 06:17:38 +0100
> Subject: [PATCH] Implement salt-value and quiet parameter for
> grub-mkpasswd-pbkdf2
>
> ---
> util/grub-mkpasswd-pbkdf2.c | 103 ++++++++++++++++++++++++++++++------
> 1 file changed, 88 insertions(+), 15 deletions(-)
>
> diff --git a/util/grub-mkpasswd-pbkdf2.c b/util/grub-mkpasswd-pbkdf2.c
> index 5805f3c10..018715bf3 100644
> --- a/util/grub-mkpasswd-pbkdf2.c
> +++ b/util/grub-mkpasswd-pbkdf2.c
> @@ -42,10 +42,15 @@
>
> #include "progname.h"
>
> +static int dec_from_hex(const int); // Converts the charcode into an integer
> representation of the representing hex value.
> +int unhexify(grub_uint8_t*, const char*);
> +
> static struct argp_option options[] = {
> {"iteration-count", 'c', N_("NUM"), 0, N_("Number of PBKDF2 iterations"),
> 0},
> {"buflen", 'l', N_("NUM"), 0, N_("Length of generated hash"), 0},
> {"salt", 's', N_("NUM"), 0, N_("Length of salt"), 0},
> + {"salt-value", 'v', N_("STR"), 0, N_("Salt"), 0},
For this patch number one...
> + {"quiet", 'q', 0, 0, N_("Only output hash, suppress other output, indended
> for pipes"), 0},
...and for this patch number two.
The rest goes into patch three. The basic idea is that one logical
change is one patch.
> { 0, 0, 0, 0, 0, 0 }
> };
>
> @@ -54,6 +59,8 @@ struct arguments
> unsigned int count;
> unsigned int buflen;
> unsigned int saltlen;
> + char *value;
> + unsigned char quiet;
> };
>
> static error_t
> @@ -76,6 +83,15 @@ argp_parser (int key, char *arg, struct argp_state *state)
> case 's':
> arguments->saltlen = strtoul (arg, NULL, 0);
> break;
> +
> + case 'v':
> + arguments->value = arg;
> + break;
> +
> + case 'q':
> + arguments->quiet = 1;
> + break;
> +
> default:
> return ARGP_ERR_UNKNOWN;
> }
> @@ -94,29 +110,70 @@ hexify (char *hex, grub_uint8_t *bin, grub_size_t n)
> {
> while (n--)
> {
> - if (((*bin & 0xf0) >> 4) < 10)
> - *hex = ((*bin & 0xf0) >> 4) + '0';
> - else
> - *hex = ((*bin & 0xf0) >> 4) + 'A' - 10;
> + if (((*bin & 0xf0) >> 4) < 10) {
> + *hex = ((*bin & 0xf0) >> 4) + '0';
> + } else {
Curly braces are not needed here and below.
> + *hex = ((*bin & 0xf0) >> 4) + 'A' - 10;
> + }
Please do not introduce clutter like this. If you wish to do some
cleanups do it in separate patch.
> hex++;
>
> if ((*bin & 0xf) < 10)
> - *hex = (*bin & 0xf) + '0';
> + *hex = (*bin & 0xf) + '0';
Ditto.
> else
> - *hex = (*bin & 0xf) + 'A' - 10;
> + *hex = (*bin & 0xf) + 'A' - 10;
Ditto.
> hex++;
> bin++;
> }
> *hex = 0;
> }
>
> +static int
> +dec_from_hex(const int hex) {
> + if (48 <= hex && hex <= 57) {
> + return hex - 48;
> + } else if (65 <= hex && hex <= 90) {
> + return hex - 65 + 10;
> + } else if (97 <= hex && hex <= 122) {
> + return hex - 97 + 10;
> + } else {
> + exit(1);
> + }
> +}
grub_strtoul() please.
> +int
> +unhexify(grub_uint8_t* output, const char* hexstring) {
> + // sizeof(output) == (sizeof(hexstring) - 1) / 2 + 1
> + if (sizeof(output) < (sizeof(hexstring) - 1) / 2 + 1) {
> + return -1;
> + }
> + char *output_ptr = NULL;
> + output_ptr = (char*)output;
> + int CharCodeInDecimal = 0;
> + void *HexDigits = xmalloc(sizeof("FF"));
> + char *HexDigitOne = HexDigits;
> + char *HexDigitTwo = (char*)HexDigits + 1;
> + char *HexString_ptr = NULL;
> + HexString_ptr = (char*)hexstring;
> + while(*HexString_ptr) {
> + memcpy (HexDigitOne, HexString_ptr++, sizeof (char));
> + memcpy (HexDigitTwo, HexString_ptr++, sizeof (char));
> +
> + CharCodeInDecimal = dec_from_hex((int)*HexDigitOne) * 16 +
> dec_from_hex((int)*HexDigitTwo);
> + *output_ptr++ = (char)CharCodeInDecimal;
> + }
> + output_ptr = '\0';
> + return 0;
> +}
This is a mess. There is a pretty good chance that similar function
exists in the wild. So, please do not reinvent the wheel.
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
