Hello,
I've separated the changes in single commits.
The patches include:
0001: Adds a quiet mode to grub-mkpasswd-pbkdf2. So that it can be used within
pipes and other tools.
0002: Suppresses the leading newline when using the quiet mode. Please pay
attention, I may have messed up the bool definition, I'm not quite sure how to
verify if that's the case.
0003: Fixes some indention and bracket placement
0004: Add a salt-value parameter to allow a caller to perform a string
comparison to check if a given password matches a given result string. This is
usefull for tools like ansible. Without it one has no possibility to use the
"has changed" functionality of ansible and needs to always set it again, as the
salt is non deterministic. Now a caller can simply run grub-mkpasswd-pbkdf2
once and copy the salt for a single password into the configuration. Therefore
the configuration management tool can recalculate the hash and perform a string
compare on the result. After that it can show that the destionation system is
within desired state and does not need to be changed.
Best,
Klaus Frank
On Wed, Jan 16, 2019 at 10:08:05PM +0100, Daniel Kiper wrote:
> 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?
I want to use ansible to set grub passwords. And I also want ansible to
recognize if a grub password is still valid. The main part of this logic will
end up within the playbook, but I'll need some functionality to make the
outcome of grub-mkpasswd-pbkdf2 deterministic, that what providing the salt and
the salt length provides. For security reasons the caller has to make sure,
that a given salt is not reused for different passwords.
And this change simultanously allows to use grub-mkpasswd-pbkdf2 within pipes:
`echo "P@ssw0rd" | grub-mkpasswd-pbkdf2 --quiet | tee /path/to/a/file`
and to validate a password by (but by using a longer salt ofcourse):
`diff <(echo 1234 | ./grub-mkpasswd-pbkdf2 --quiet
--salt-value=D2F60AAE43405216C9C6 -s 10) <(cat /some/file.txt)`
>
> > 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
>From d49d6aaaa59fe0f9853171b5cd9963dbedb37fa7 Mon Sep 17 00:00:00 2001
From: Klaus Frank <g...@frank.fyi>
Date: Sat, 19 Jan 2019 19:03:40 +0100
Subject: [PATCH 1/4] Add quiet mode
---
util/grub-mkpasswd-pbkdf2.c | 32 ++++++++++++++++++++++++--------
1 file changed, 24 insertions(+), 8 deletions(-)
diff --git a/util/grub-mkpasswd-pbkdf2.c b/util/grub-mkpasswd-pbkdf2.c
index 5805f3c10..ceb8570bb 100644
--- a/util/grub-mkpasswd-pbkdf2.c
+++ b/util/grub-mkpasswd-pbkdf2.c
@@ -46,6 +46,7 @@ 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},
+ {"quiet", 'q', 0, 0, N_("Only output hash, suppress other output, indended for pipes"), 0},
{ 0, 0, 0, 0, 0, 0 }
};
@@ -54,6 +55,7 @@ struct arguments
unsigned int count;
unsigned int buflen;
unsigned int saltlen;
+ unsigned char quiet;
};
static error_t
@@ -76,6 +78,11 @@ argp_parser (int key, char *arg, struct argp_state *state)
case 's':
arguments->saltlen = strtoul (arg, NULL, 0);
break;
+
+ case 'q':
+ arguments->quiet = 1;
+ break;
+
default:
return ARGP_ERR_UNKNOWN;
}
@@ -116,7 +123,8 @@ main (int argc, char *argv[])
struct arguments arguments = {
.count = 10000,
.buflen = 64,
- .saltlen = 64
+ .saltlen = 64,
+ .quiet = 0
};
char *result, *ptr;
gcry_err_code_t gcry_err;
@@ -135,23 +143,26 @@ main (int argc, char *argv[])
buf = xmalloc (arguments.buflen);
salt = xmalloc (arguments.saltlen);
-
- printf ("%s", _("Enter password: "));
+
+ if (!arguments.quiet) {
+ printf ("%s", _("Enter password: "));
+ }
if (!grub_password_get (pass1, GRUB_AUTH_MAX_PASSLEN))
{
free (buf);
free (salt);
grub_util_error ("%s", _("failure to read password"));
}
- printf ("%s", _("Reenter password: "));
- if (!grub_password_get (pass2, GRUB_AUTH_MAX_PASSLEN))
+ if (!arguments.quiet) {
+ printf ("%s", _("Reenter password: "));
+ if (!grub_password_get (pass2, GRUB_AUTH_MAX_PASSLEN))
{
free (buf);
free (salt);
grub_util_error ("%s", _("failure to read password"));
}
- if (strcmp (pass1, pass2) != 0)
+ if (strcmp (pass1, pass2) != 0)
{
memset (pass1, 0, sizeof (pass1));
memset (pass2, 0, sizeof (pass2));
@@ -159,7 +170,8 @@ main (int argc, char *argv[])
free (salt);
grub_util_error ("%s", _("passwords don't match"));
}
- memset (pass2, 0, sizeof (pass2));
+ memset (pass2, 0, sizeof (pass2));
+ }
if (grub_get_random (salt, arguments.saltlen))
{
@@ -200,7 +212,11 @@ main (int argc, char *argv[])
ptr += arguments.buflen * 2;
*ptr = '\0';
- printf (_("PBKDF2 hash of your password is %s\n"), result);
+ if (arguments.quiet) {
+ printf ("%s\n", result);
+ } else {
+ printf (_("PBKDF2 hash of your password is %s\n"), result);
+ }
memset (buf, 0, arguments.buflen);
free (buf);
memset (salt, 0, arguments.saltlen);
--
2.20.1
>From 823a482178577c8c89618e1e537bc5401fbb8e30 Mon Sep 17 00:00:00 2001
From: Klaus Frank <g...@frank.fyi>
Date: Sun, 20 Jan 2019 01:13:34 +0100
Subject: [PATCH 2/4] Suppress newline in quiet mode
---
grub-core/lib/crypto.c | 12 ++++++++++--
grub-core/osdep/unix/password.c | 12 ++++++++++--
grub-core/osdep/windows/password.c | 11 ++++++++++-
include/grub/crypto.h | 8 ++++++++
util/grub-mkpasswd-pbkdf2.c | 4 ++--
5 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/grub-core/lib/crypto.c b/grub-core/lib/crypto.c
index ca334d5a4..50c800af5 100644
--- a/grub-core/lib/crypto.c
+++ b/grub-core/lib/crypto.c
@@ -451,7 +451,7 @@ grub_crypto_memcmp (const void *a, const void *b, grub_size_t n)
#ifndef GRUB_UTIL
int
-grub_password_get (char buf[], unsigned buf_size)
+_grub_password_get (char buf[], unsigned buf_size, bool newline)
{
unsigned cur_len = 0;
int key;
@@ -484,10 +484,18 @@ grub_password_get (char buf[], unsigned buf_size)
grub_memset (buf + cur_len, 0, buf_size - cur_len);
- grub_xputs ("\n");
+ if (newline) {
+ grub_xputs ("\n");
+ }
grub_refresh ();
return (key != GRUB_TERM_ESC);
}
+
+int
+grub_password_get (char buf[], unsigned buf_size)
+{
+ return _grub_password_get(buf, buf_size, true);
+}
#endif
diff --git a/grub-core/osdep/unix/password.c b/grub-core/osdep/unix/password.c
index 9996b244b..5c3a2b021 100644
--- a/grub-core/osdep/unix/password.c
+++ b/grub-core/osdep/unix/password.c
@@ -27,7 +27,7 @@
#include <stdio.h>
int
-grub_password_get (char buf[], unsigned buf_size)
+_grub_password_get (char buf[], unsigned buf_size, bool newline)
{
FILE *in;
struct termios s, t;
@@ -65,7 +65,9 @@ grub_password_get (char buf[], unsigned buf_size)
if (tty_changed)
(void) tcsetattr (fileno (in), TCSAFLUSH, &s);
- grub_xputs ("\n");
+ if (newline) {
+ grub_xputs ("\n");
+ }
grub_refresh ();
if (in != stdin)
@@ -73,3 +75,9 @@ grub_password_get (char buf[], unsigned buf_size)
return 1;
}
+
+int
+grub_password_get (char buf[], unsigned buf_size)
+{
+ return _grub_password_get(buf, buf_size, true);
+}
diff --git a/grub-core/osdep/windows/password.c b/grub-core/osdep/windows/password.c
index 1d3af0c2c..79eafb4fa 100644
--- a/grub-core/osdep/windows/password.c
+++ b/grub-core/osdep/windows/password.c
@@ -27,7 +27,7 @@
#include <stdio.h>
int
-grub_password_get (char buf[], unsigned buf_size)
+_grub_password_get (char buf[], unsigned buf_size, bool newline)
{
HANDLE hStdin = GetStdHandle (STD_INPUT_HANDLE);
DWORD mode = 0;
@@ -45,7 +45,16 @@ grub_password_get (char buf[], unsigned buf_size)
SetConsoleMode (hStdin, mode);
+ if (newline) {
+ grub_xputs ("\n");
+ }
grub_refresh ();
return 1;
}
+
+int
+grub_password_get (char buf[], unsigned buf_size)
+{
+ return _grub_password_get(buf, buf_size, false);
+}
diff --git a/include/grub/crypto.h b/include/grub/crypto.h
index a24e89dd9..9cdf0693f 100644
--- a/include/grub/crypto.h
+++ b/include/grub/crypto.h
@@ -28,6 +28,11 @@
#include <grub/err.h>
#include <grub/mm.h>
+#ifndef GRUB_POSIX_BOOL_DEFINED
+#define GRUB_POSIX_BOOL_DEFINED 1
+#include <stdbool.h>
+#endif
+
typedef enum
{
GPG_ERR_NO_ERROR,
@@ -396,6 +401,9 @@ grub_crypto_pbkdf2 (const struct gcry_md_spec *md,
int
grub_crypto_memcmp (const void *a, const void *b, grub_size_t n);
+int
+_grub_password_get (char buf[], unsigned buf_size, bool newline);
+
int
grub_password_get (char buf[], unsigned buf_size);
diff --git a/util/grub-mkpasswd-pbkdf2.c b/util/grub-mkpasswd-pbkdf2.c
index ceb8570bb..66f3ab2ba 100644
--- a/util/grub-mkpasswd-pbkdf2.c
+++ b/util/grub-mkpasswd-pbkdf2.c
@@ -147,14 +147,14 @@ main (int argc, char *argv[])
if (!arguments.quiet) {
printf ("%s", _("Enter password: "));
}
- if (!grub_password_get (pass1, GRUB_AUTH_MAX_PASSLEN))
+ if (!_grub_password_get (pass1, GRUB_AUTH_MAX_PASSLEN, false))
{
free (buf);
free (salt);
grub_util_error ("%s", _("failure to read password"));
}
if (!arguments.quiet) {
- printf ("%s", _("Reenter password: "));
+ printf ("%s", _("\nReenter password: "));
if (!grub_password_get (pass2, GRUB_AUTH_MAX_PASSLEN))
{
free (buf);
--
2.20.1
>From 8d1bda7fbc3d19fedec46a043adb81720a177103 Mon Sep 17 00:00:00 2001
From: Klaus Frank <g...@frank.fyi>
Date: Sat, 19 Jan 2019 19:23:25 +0100
Subject: [PATCH 3/4] Fix indention and brackets, to prevent failures like the
double goto fail from apple.
---
util/grub-mkpasswd-pbkdf2.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/util/grub-mkpasswd-pbkdf2.c b/util/grub-mkpasswd-pbkdf2.c
index 66f3ab2ba..92e18fd37 100644
--- a/util/grub-mkpasswd-pbkdf2.c
+++ b/util/grub-mkpasswd-pbkdf2.c
@@ -101,16 +101,18 @@ 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 {
+ *hex = ((*bin & 0xf0) >> 4) + 'A' - 10;
+ }
hex++;
- if ((*bin & 0xf) < 10)
- *hex = (*bin & 0xf) + '0';
- else
- *hex = (*bin & 0xf) + 'A' - 10;
+ if ((*bin & 0xf) < 10) {
+ *hex = (*bin & 0xf) + '0';
+ } else {
+ *hex = (*bin & 0xf) + 'A' - 10;
+ }
hex++;
bin++;
}
--
2.20.1
>From f06f4c0388797a50b55f479f7b10a24a52be4701 Mon Sep 17 00:00:00 2001
From: Klaus Frank <g...@frank.fyi>
Date: Sun, 20 Jan 2019 00:03:13 +0100
Subject: [PATCH 4/4] add verify password functionality
---
util/grub-mkpasswd-pbkdf2.c | 33 ++++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/util/grub-mkpasswd-pbkdf2.c b/util/grub-mkpasswd-pbkdf2.c
index 92e18fd37..4ed62d04f 100644
--- a/util/grub-mkpasswd-pbkdf2.c
+++ b/util/grub-mkpasswd-pbkdf2.c
@@ -42,10 +42,13 @@
#include "progname.h"
+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},
{"quiet", 'q', 0, 0, N_("Only output hash, suppress other output, indended for pipes"), 0},
{ 0, 0, 0, 0, 0, 0 }
};
@@ -55,6 +58,7 @@ struct arguments
unsigned int count;
unsigned int buflen;
unsigned int saltlen;
+ char *value;
unsigned char quiet;
};
@@ -79,6 +83,10 @@ argp_parser (int key, char *arg, struct argp_state *state)
arguments->saltlen = strtoul (arg, NULL, 0);
break;
+ case 'v':
+ arguments->value = arg;
+ break;
+
case 'q':
arguments->quiet = 1;
break;
@@ -119,6 +127,19 @@ hexify (char *hex, grub_uint8_t *bin, grub_size_t n)
*hex = 0;
}
+int
+unhexify(grub_uint8_t* output, const char* hexstring) {
+ if (sizeof(output) < (sizeof(hexstring) - 1) / 2 + 1) {
+ return -1;
+ }
+ char *output_ptr = (char*)output;
+ char *hexstring_ptr = (char*)hexstring;
+ for (int c; hexstring[0] && hexstring[1] && sscanf(hexstring_ptr, "%2x", &c); hexstring_ptr += 2) {
+ sprintf(output_ptr++, "%s",(char*)&c);
+ }
+ return 0;
+}
+
int
main (int argc, char *argv[])
{
@@ -126,6 +147,7 @@ main (int argc, char *argv[])
.count = 10000,
.buflen = 64,
.saltlen = 64,
+ .value = NULL,
.quiet = 0
};
char *result, *ptr;
@@ -175,13 +197,22 @@ main (int argc, char *argv[])
memset (pass2, 0, sizeof (pass2));
}
- if (grub_get_random (salt, arguments.saltlen))
+ if (arguments.value) {
+ if (unhexify(salt, arguments.value)) {
+ memset(pass1, 0, sizeof(pass1));
+ free(buf);
+ free(salt);
+ grub_util_error("%s", _("couldn't convert hexstring into salt"));
+ }
+ } else {
+ if (grub_get_random (salt, arguments.saltlen))
{
memset (pass1, 0, sizeof (pass1));
free (buf);
free (salt);
grub_util_error ("%s", _("couldn't retrieve random data for salt"));
}
+ }
gcry_err = grub_crypto_pbkdf2 (GRUB_MD_SHA512,
(grub_uint8_t *) pass1, strlen (pass1),
--
2.20.1
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
