Re: [PATCH v4 2/2] luks2: Fix decoding of digests and salts with escaped chars
On Thu, Jun 30, 2022 at 06:05:12PM +0200, Daniel Kiper wrote:
> On Mon, Jun 06, 2022 at 07:29:00AM +0200, Patrick Steinhardt wrote:
> > It was reported in the #grub IRC channel on Libera that decryption of
> > LUKS2 partitions fails with errors about invalid digests and/or salts.
> > In all of these cases, what failed was decoding the Base64
> > representation of these, where the encoded data contained invalid
> > characters.
> >
> > As it turns out, the root cause is that json-c, which is used by
> > cryptsetup to read and write the JSON header, will escape some
> > characters by prepending a backslash when writing JSON strings by
> > default. Most importantly, json-c also escapes the forward slash, which
> > is part of the Base64 alphabet. Because GRUB doesn't know to unescape
> > such characters, decoding this string will rightfully fail.
> >
> > Interestingly, this issue has until now only been reported by users of
> > Ubuntu 18.04. And a bit of digging in fact reveals that cryptsetup has
> > changed the logic in a054206d (Suppress useless slash escaping in json
> > lib, 2018-04-20), which has been released with cryptsetup v2.0.3. Ubuntu
> > 18.04 is still shipping with cryptsetup v2.0.2 though, which explains
> > why this is not a more frequent issue.
> >
> > Fix the issue by using our new `grub_json_unescape ()` helper function
> > that handles unescaping for us.
> >
> > Reported-by: Afdal
> > Signed-off-by: Patrick Steinhardt
>
> I think this patch should be merged with the first one. Of course it
> requires similar fixes mentioned in my earlier reply. Plus a few
> additional ones mentioned below...
I think it's easier to review this as two independent atomic changes.
The decoding logic is complex enough to warrant its own patch given it
already requires some focus to properly review, and the second patch has
historic context around cryptsetup that's not relevant to JSON itself.
I'll send v5 with separate patches, but I'll change it if you insist.
> > grub-core/disk/luks2.c | 28
> > 1 file changed, 24 insertions(+), 4 deletions(-)
> >
> > diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> > index bf741d70f..728f93a8c 100644
> > --- a/grub-core/disk/luks2.c
> > +++ b/grub-core/disk/luks2.c
> > @@ -384,6 +384,24 @@ luks2_scan (grub_disk_t disk, grub_cryptomount_args_t
> > cargs)
> >return cryptodisk;
> > }
> >
> > +static grub_err_t
> > +luks2_base64_decode (const char *in, grub_size_t inlen, grub_uint8_t
> > *decoded, idx_t *decodedlen)
> > +{
> > + grub_size_t unescaped_len = 0;
> > + char *unescaped = NULL;
> > + bool successful;
> > +
> > + if (grub_json_unescape (&unescaped, &unescaped_len, in, inlen) !=
> > GRUB_ERR_NONE)
> > +return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not unescape Base64
> > string");
> > +
> > + successful = base64_decode (unescaped, (size_t)unescaped_len, (char
> > *)decoded, decodedlen);
>
> s/(char *)decoded/(char *) decoded/
>
> You need an additional space here.
>
> s/(size_t)unescaped_len/(size_t) unescaped_len/
>
> s/size_t/grub_size_t/? Hmmm... It is idx_t in declaration...
> Do we need cast here at all?
Yeah, should be `grub_size_t`. But the cast is required, isn't it?
`idx_t` is a typedef of `ptrdiff_t`, which is signed, and `grub_size_t`
is unsigned.
> > + grub_free (unescaped);
> > + if (!successful)
> > +return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not decode Base64
> > string");
> > +
> > + return GRUB_ERR_NONE;
> > +}
> > +
> > static grub_err_t
> > luks2_verify_key (grub_luks2_digest_t *d, grub_uint8_t *candidate_key,
> > grub_size_t candidate_key_len)
> > @@ -395,9 +413,11 @@ luks2_verify_key (grub_luks2_digest_t *d, grub_uint8_t
> > *candidate_key,
> >gcry_err_code_t gcry_ret;
> >
> >/* Decode both digest and salt */
> > - if (!base64_decode (d->digest, grub_strlen (d->digest), (char *)digest,
> > &digestlen))
> > + if (luks2_base64_decode (d->digest, grub_strlen (d->digest),
> > + digest, &digestlen) != GRUB_ERR_NONE)
> > return grub_error (GRUB_ERR_BAD_ARGUMENT, "Invalid digest");
> > - if (!base64_decode (d->salt, grub_strlen (d->salt), (char *)salt,
> > &saltlen))
> > + if (luks2_base64_decode (d->salt, grub_strlen (d->salt),
> > + salt, &saltlen) != GRUB_ERR_NONE)
> > return grub_error (GRUB_ERR_BAD_ARGUMENT, "Invalid digest salt");
> >
> >/* Configure the hash used for the digest. */
> > @@ -435,8 +455,8 @@ luks2_decrypt_key (grub_uint8_t *out_key,
> >gcry_err_code_t gcry_ret;
> >grub_err_t ret;
> >
> > - if (!base64_decode (k->kdf.salt, grub_strlen (k->kdf.salt),
> > -(char *)salt, &saltlen))
> > + if (luks2_base64_decode (k->kdf.salt, grub_strlen (k->kdf.salt),
> > + salt, &saltlen) != GRUB_ERR_NONE)
> > {
> >ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Invalid keyslot salt");
> >goto err;
>
> Daniel
Patrick
[PATCH v5 0/2] luks2: Fix decoding of digests and salts with escaped chars
Hi,
this is the fifth version of my patch series which fixes decoding of
digests and salts in LUKS2 headers in case they happen to contain
escaped characters. While modern cryptsetup versions in fact don't
escape any characters part of the Base64 alphabet, old versions of
cryptsetup did this until v2.0.2.
Changes compared to v4 include mostly style-related fixes pointed out by
Daniel. Please refer to the range-diff below.
Patrick
Patrick Steinhardt (2):
json: Add function to unescape JSON-encoded strings
luks2: Fix decoding of digests and salts with escaped chars
grub-core/disk/luks2.c| 28 +++--
grub-core/lib/json/json.c | 118 ++
grub-core/lib/json/json.h | 12
3 files changed, 154 insertions(+), 4 deletions(-)
Range-diff against v4:
1: c2233323a ! 1: ebab6b092 json: Add function to unescape JSON-encoded
strings
@@ grub-core/lib/json/json.c: grub_json_getint64 (grub_int64_t *out, const
grub_jso
+ grub_size_t inpos, resultpos;
+ char *result;
+
-+ if (!out || !outlen)
-+return grub_error (GRUB_ERR_BAD_ARGUMENT, "Output parameters are not
set");
++ if (out == NULL || outlen == NULL)
++return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("output parameters are
not set"));
+
+ result = grub_calloc (1, inlen + 1);
-+ if (!result)
++ if (result == NULL)
+return GRUB_ERR_OUT_OF_MEMORY;
+
+ for (inpos = resultpos = 0; inpos < inlen; inpos++)
@@ grub-core/lib/json/json.c: grub_json_getint64 (grub_int64_t *out, const
grub_jso
+inpos++;
+if (inpos >= inlen)
+ {
-+ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Expected escaped
character");
++ret = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("expected escaped
character"));
+goto err;
+ }
+
+switch (in[inpos])
+ {
+case '"':
-+ result[resultpos++] = '"'; break;
++ result[resultpos++] = '"';
++ break;
++
+case '/':
-+ result[resultpos++] = '/'; break;
++ result[resultpos++] = '/';
++ break;
++
+case '\\':
-+ result[resultpos++] = '\\'; break;
++ result[resultpos++] = '\\';
++ break;
++
+case 'b':
-+ result[resultpos++] = '\b'; break;
++ result[resultpos++] = '\b';
++ break;
++
+case 'f':
-+ result[resultpos++] = '\f'; break;
++ result[resultpos++] = '\f';
++ break;
++
+case 'r':
-+ result[resultpos++] = '\r'; break;
++ result[resultpos++] = '\r';
++ break;
++
+case 'n':
-+ result[resultpos++] = '\n'; break;
++ result[resultpos++] = '\n';
++ break;
++
+case 't':
-+ result[resultpos++] = '\t'; break;
++ result[resultpos++] = '\t';
++ break;
++
+case 'u':
+ {
-+unsigned char values[4] = {0};
-+int i;
++char values[4] = {0};
++unsigned i;
+
+inpos++;
+if (inpos + ARRAY_SIZE(values) > inlen)
+ {
-+ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unicode
sequence too short");
++ret = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("unicode
sequence too short"));
+goto err;
+ }
+
-+for (i = 0; i < 4; i++)
++for (i = 0; i < ARRAY_SIZE(values); i++)
+ {
+char c = in[inpos++];
+
@@ grub-core/lib/json/json.c: grub_json_getint64 (grub_int64_t *out, const
grub_jso
+else
+ {
+ret = grub_error (GRUB_ERR_BAD_ARGUMENT,
-+ "Unicode sequence with invalid
character '%c'", c);
++ N_("unicode sequence with invalid
character '%c'"), c);
+goto err;
+ }
+ }
@@ grub-core/lib/json/json.c: grub_json_getint64 (grub_int64_t *out, const
grub_jso
+
+break;
+ }
++
+default:
-+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unrecognized escaped
character '%c'", in[inpos]);
++ ret = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("unrecognized
escaped character '%c'"), in[inpos]);
+ goto err;
+ }
+ }
@@ grub-core/lib/json/json.c: grub_json_getint64 (grub_int64_t *out, const
grub_jso
+ *out = result;
+ *outlen = resultpos;
+
-+err:
++ err:
+ if (ret != GRUB_ERR_NONE)
+grub_free
[PATCH v5 1/2] json: Add function to unescape JSON-encoded strings
JSON strings require certain characters to be encoded, either by using a
single reverse solidus character "\" for a set of popular characters, or
by using a Unicode representation of "\uX". The jsmn library doesn't
handle unescaping for us, so we must implement this functionality for
ourselves.
Add a new function `grub_json_unescape ()` that takes a potentially
escaped JSON string as input and returns a new unescaped string.
Signed-off-by: Patrick Steinhardt
---
grub-core/lib/json/json.c | 118 ++
grub-core/lib/json/json.h | 12
2 files changed, 130 insertions(+)
diff --git a/grub-core/lib/json/json.c b/grub-core/lib/json/json.c
index 1c20c75ea..1eadd1ce9 100644
--- a/grub-core/lib/json/json.c
+++ b/grub-core/lib/json/json.c
@@ -262,3 +262,121 @@ grub_json_getint64 (grub_int64_t *out, const grub_json_t
*parent, const char *ke
return GRUB_ERR_NONE;
}
+
+grub_err_t
+grub_json_unescape (char **out, grub_size_t *outlen, const char *in,
grub_size_t inlen)
+{
+ grub_err_t ret = GRUB_ERR_NONE;
+ grub_size_t inpos, resultpos;
+ char *result;
+
+ if (out == NULL || outlen == NULL)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("output parameters are not
set"));
+
+ result = grub_calloc (1, inlen + 1);
+ if (result == NULL)
+return GRUB_ERR_OUT_OF_MEMORY;
+
+ for (inpos = resultpos = 0; inpos < inlen; inpos++)
+{
+ if (in[inpos] == '\\')
+ {
+ inpos++;
+ if (inpos >= inlen)
+ {
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("expected escaped
character"));
+ goto err;
+ }
+
+ switch (in[inpos])
+ {
+ case '"':
+ result[resultpos++] = '"';
+ break;
+
+ case '/':
+ result[resultpos++] = '/';
+ break;
+
+ case '\\':
+ result[resultpos++] = '\\';
+ break;
+
+ case 'b':
+ result[resultpos++] = '\b';
+ break;
+
+ case 'f':
+ result[resultpos++] = '\f';
+ break;
+
+ case 'r':
+ result[resultpos++] = '\r';
+ break;
+
+ case 'n':
+ result[resultpos++] = '\n';
+ break;
+
+ case 't':
+ result[resultpos++] = '\t';
+ break;
+
+ case 'u':
+ {
+ char values[4] = {0};
+ unsigned i;
+
+ inpos++;
+ if (inpos + ARRAY_SIZE(values) > inlen)
+ {
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("unicode
sequence too short"));
+ goto err;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(values); i++)
+ {
+ char c = in[inpos++];
+
+ if (c >= '0' && c <= '9')
+ values[i] = c - '0';
+ else if (c >= 'A' && c <= 'F')
+ values[i] = c - 'A' + 10;
+ else if (c >= 'a' && c <= 'f')
+ values[i] = c - 'a' + 10;
+ else
+ {
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT,
+ N_("unicode sequence with invalid
character '%c'"), c);
+ goto err;
+ }
+ }
+
+ if (values[0] != 0 || values[1] != 0)
+ result[resultpos++] = values[0] << 4 | values[1];
+ result[resultpos++] = values[2] << 4 | values[3];
+
+ /* Offset the increment that's coming in via the loop
increment. */
+ inpos--;
+
+ break;
+ }
+
+ default:
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("unrecognized
escaped character '%c'"), in[inpos]);
+ goto err;
+ }
+ }
+ else
+ result[resultpos++] = in[inpos];
+}
+
+ *out = result;
+ *outlen = resultpos;
+
+ err:
+ if (ret != GRUB_ERR_NONE)
+grub_free (result);
+
+ return ret;
+}
diff --git a/grub-core/lib/json/json.h b/grub-core/lib/json/json.h
index 4ea2a22d8..626074c35 100644
--- a/grub-core/lib/json/json.h
+++ b/grub-core/lib/json/json.h
@@ -125,4 +125,16 @@ extern grub_err_t EXPORT_FUNC(grub_json_getint64)
(grub_int64_t *out,
const grub_json_t *parent,
const char *key);
+/*
+ * Unescape escaped characters and Unicode sequences in the
+ * given JSON-encoded string. Returns a newly allocated string
+ * passed back via the `out` parameter that has a length of
+ * `*outlen`.
+ *
+ * See https://datatracker.ietf.org/doc/html/rfc8259#section-7 for more
+ * information on escaping in JSON.
+ */
+exte
[PATCH v5 2/2] luks2: Fix decoding of digests and salts with escaped chars
It was reported in the #grub IRC channel on Libera that decryption of
LUKS2 partitions fails with errors about invalid digests and/or salts.
In all of these cases, what failed was decoding the Base64
representation of these, where the encoded data contained invalid
characters.
As it turns out, the root cause is that json-c, which is used by
cryptsetup to read and write the JSON header, will escape some
characters by prepending a backslash when writing JSON strings by
default. Most importantly, json-c also escapes the forward slash, which
is part of the Base64 alphabet. Because GRUB doesn't know to unescape
such characters, decoding this string will rightfully fail.
Interestingly, this issue has until now only been reported by users of
Ubuntu 18.04. And a bit of digging in fact reveals that cryptsetup has
changed the logic in a054206d (Suppress useless slash escaping in json
lib, 2018-04-20), which has been released with cryptsetup v2.0.3. Ubuntu
18.04 is still shipping with cryptsetup v2.0.2 though, which explains
why this is not a more frequent issue.
Fix the issue by using our new `grub_json_unescape ()` helper function
that handles unescaping for us.
Reported-by: Afdal
Signed-off-by: Patrick Steinhardt
---
grub-core/disk/luks2.c | 28
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
index bf741d70f..c24c6e98d 100644
--- a/grub-core/disk/luks2.c
+++ b/grub-core/disk/luks2.c
@@ -384,6 +384,24 @@ luks2_scan (grub_disk_t disk, grub_cryptomount_args_t
cargs)
return cryptodisk;
}
+static grub_err_t
+luks2_base64_decode (const char *in, grub_size_t inlen, grub_uint8_t *decoded,
idx_t *decodedlen)
+{
+ grub_size_t unescaped_len = 0;
+ char *unescaped = NULL;
+ bool successful;
+
+ if (grub_json_unescape (&unescaped, &unescaped_len, in, inlen) !=
GRUB_ERR_NONE)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("could not unescape Base64
string"));
+
+ successful = base64_decode (unescaped, (grub_size_t) unescaped_len, (char *)
decoded, decodedlen);
+ grub_free (unescaped);
+ if (!successful)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("could not decode Base64
string"));
+
+ return GRUB_ERR_NONE;
+}
+
static grub_err_t
luks2_verify_key (grub_luks2_digest_t *d, grub_uint8_t *candidate_key,
grub_size_t candidate_key_len)
@@ -395,9 +413,11 @@ luks2_verify_key (grub_luks2_digest_t *d, grub_uint8_t
*candidate_key,
gcry_err_code_t gcry_ret;
/* Decode both digest and salt */
- if (!base64_decode (d->digest, grub_strlen (d->digest), (char *)digest,
&digestlen))
+ if (luks2_base64_decode (d->digest, grub_strlen (d->digest),
+ digest, &digestlen) != GRUB_ERR_NONE)
return grub_error (GRUB_ERR_BAD_ARGUMENT, "Invalid digest");
- if (!base64_decode (d->salt, grub_strlen (d->salt), (char *)salt, &saltlen))
+ if (luks2_base64_decode (d->salt, grub_strlen (d->salt),
+ salt, &saltlen) != GRUB_ERR_NONE)
return grub_error (GRUB_ERR_BAD_ARGUMENT, "Invalid digest salt");
/* Configure the hash used for the digest. */
@@ -435,8 +455,8 @@ luks2_decrypt_key (grub_uint8_t *out_key,
gcry_err_code_t gcry_ret;
grub_err_t ret;
- if (!base64_decode (k->kdf.salt, grub_strlen (k->kdf.salt),
-(char *)salt, &saltlen))
+ if (luks2_base64_decode (k->kdf.salt, grub_strlen (k->kdf.salt),
+ salt, &saltlen) != GRUB_ERR_NONE)
{
ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Invalid keyslot salt");
goto err;
--
2.37.0
signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v5 1/2] json: Add function to unescape JSON-encoded strings
On 7/11/22 06:44, Patrick Steinhardt wrote:
JSON strings require certain characters to be encoded, either by using a
single reverse solidus character "\" for a set of popular characters, or
by using a Unicode representation of "\uX". The jsmn library doesn't
handle unescaping for us, so we must implement this functionality for
ourselves.
Add a new function `grub_json_unescape ()` that takes a potentially
escaped JSON string as input and returns a new unescaped string.
Signed-off-by: Patrick Steinhardt
---
grub-core/lib/json/json.c | 118 ++
grub-core/lib/json/json.h | 12
2 files changed, 130 insertions(+)
diff --git a/grub-core/lib/json/json.c b/grub-core/lib/json/json.c
index 1c20c75ea..1eadd1ce9 100644
--- a/grub-core/lib/json/json.c
+++ b/grub-core/lib/json/json.c
@@ -262,3 +262,121 @@ grub_json_getint64 (grub_int64_t *out, const grub_json_t
*parent, const char *ke
return GRUB_ERR_NONE;
}
+
+grub_err_t
+grub_json_unescape (char **out, grub_size_t *outlen, const char *in,
grub_size_t inlen)
+{
+ grub_err_t ret = GRUB_ERR_NONE;
+ grub_size_t inpos, resultpos;
+ char *result;
+
+ if (out == NULL || outlen == NULL)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("output parameters are not
set"));
+
+ result = grub_calloc (1, inlen + 1);
+ if (result == NULL)
+return GRUB_ERR_OUT_OF_MEMORY;
+
+ for (inpos = resultpos = 0; inpos < inlen; inpos++)
+{
+ if (in[inpos] == '\\')
+ {
+ inpos++;
+ if (inpos >= inlen)
+ {
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("expected escaped
character"));
+ goto err;
+ }
+
+ switch (in[inpos])
+ {
+ case '"':
+ result[resultpos++] = '"';
+ break;
+
+ case '/':
+ result[resultpos++] = '/';
+ break;
+
+ case '\\':
+ result[resultpos++] = '\\';
+ break;
+
+ case 'b':
+ result[resultpos++] = '\b';
+ break;
+
+ case 'f':
+ result[resultpos++] = '\f';
+ break;
+
+ case 'r':
+ result[resultpos++] = '\r';
+ break;
+
+ case 'n':
+ result[resultpos++] = '\n';
+ break;
+
+ case 't':
+ result[resultpos++] = '\t';
+ break;
+
+ case 'u':
+ {
+ char values[4] = {0};
+ unsigned i;
+
+ inpos++;
+ if (inpos + ARRAY_SIZE(values) > inlen)
+ {
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("unicode sequence
too short"));
+ goto err;
+ }
I recommend relaxing these errors a little bit. While technically
correct, I'm thinking it might serve GRUB better to be a bit more
forgiving.
Perhaps something like:
if the input is '\u' where ? is not a hex digit, set the value to
'u', and process separately.
if is less than ARRAY_SIZE chars, left-pad with 0s* until it's
the correct length then convert the value.
(* I don't mean to literally pad. I mean to treat the short number as if
it was left-padded with 0s)
+
+ for (i = 0; i < ARRAY_SIZE(values); i++)
+ {
+ char c = in[inpos++];
+
+ if (c >= '0' && c <= '9')
+ values[i] = c - '0';
+ else if (c >= 'A' && c <= 'F')
+ values[i] = c - 'A' + 10;
+ else if (c >= 'a' && c <= 'f')
+ values[i] = c - 'a' + 10;
+ else
+ {
With a similar argument here. Treat the short as suggested above
and resume normal processing at the non-hex digit position.
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT,
+ N_("unicode sequence with invalid
character '%c'"), c);
+ goto err;
+ }
+ }
+
+ if (values[0] != 0 || values[1] != 0)
+ result[resultpos++] = values[0] << 4 | values[1];
+ result[resultpos++] = values[2] << 4 | values[3];
+
+ /* Offset the increment that's coming in via the loop
increment. */
+ inpos--;
+
+ break;
+ }
+
+ default:
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("unrecognized escaped
character '%c'"), in[inpos]);
+ goto err;
And the final prong of the suggestion would be here. Treat the improper
escape as if it had not been escaped at all or as '\\?' where '?' is the
escaped character.
Thanks,
Nicholas Vinson
+ }
+ }
+ else
+ result[resultpos++] = in[i
[PATCH v4 1/1] plainmount: Support plain encryption mode
--- Original Message ---
On Sunday, July 10th, 2022 at 9:07 PM, Glenn Washburn
wrote:
> > +
> > +plainmount hd0,gpt1 -o 1048576
> > +
> > +
> > +both create virtual devices with 1MiB offset on top of the specified
> > partition. The
> > +option @option{-o} is useful to specify offset which is not aligned to 512
> > byte
>
>
> I just noticed that the code takes this parameter and turns it into a
> number of sectors that the data is offset from the start of the device.
> Thus internally this gets rounded down to be sector aligned anyway. In
> fact, its aligned to the size of the sector specified with the -S
> option. So we should get rid of the -o entirely. In fact, -o will not
> work if it is not a multiple of the sector size specified with -S.
Option -o is taken as a byte number, not as a sector number, it is not
multiplied
by 512 or by sector size. However later, indeed it is truncated to a sector
size,
loosing its meaning. I agree with the rest of the comment, this option is
irrelevant
and should be removed.
>
> > +sector size. Note: current cryptsetup implementation of plain mode and
> > LUKS v1 restricts
> > +alignment to 512 byte sector size. Specifying arbitrary byte alignment is
> > useful only to
> > +open LUKS v2 volume if master key is known or to open the volume encrypted
> > by other
> > +cryptographic implementation. Note: in LUKS revealing master key is not
> > recommended
> > +because it allows to open encrypted device directly bypassing the header
> > and LUKS
> > +security features.
>
>
> These "Notes" should probably be reorganized as @footnote{Put note
> inside of this.} You have two notes here, so perhaps two foot notes,
> although it looks like here you have a nested note. I'm not sure if you
> can have nested footnotes.
I can change them to be just a single note.
> > +
> > +Password can be specified in two ways: as a password data from a keyfile
> > or as a secret
> > +passphrase. The keyfile parameter @option{-d} specifies path to the
> > keyfile containing
> > +password data and has higher priority than the other password method.
> > Specified password
> > +data in this mode is not hashed. The option @option{-O} specifies offset
> > in terms of bytes
>
>
> Hmm its not hashed? I thought a keyfile that contains 4 characters
> "pass" would be the same as entering "pass" as the secret passphrase.
> If the keyfile is not hashed, then the derived master key is different.
> Are you Am I mistaken or missing something?
I have taken this behavior from cryptsetup. Having binary key file data
directly allows
to have arbitrary password (not necessarily typable with a keyboard). Such
feature is not
very popular, but it exists. This feature is connected with use cases where
user types
password in non-English keyboard (possibly, a letter which is not easily
encoded in UTF,
or which encoding depends on keyboard layout), which cannot be typed at another
computer.
> > +of the password data from the start of the keyfile. This option has no
> > effect without the
> > +option @option{-d}. Offset of password data in the keyfile can be
> > specified directly with
> > +option @option{-d} and GRUB blocklist syntax (for example: '-d
> > (hd0,gpt1)2048+').
>
>
> Should this be -O instead of -d?
Why? The last part of quoted text says that alternative to option -O is the
blocklist syntax
which is part of the -d option.
> > +configured when creating the encrypted volume. Attempting to decrypt such
> > volume with
> > +different sector size than it was created with will not result in an
> > error, but will
> > +decrypt to random bytes and thus prevent accessing the volume.
>
>
> Actually, some sectors will be decrypted correctly. So perhaps we
> should say "but not all sectors will be properly decrypted, generally
> causing the volume to be inaccessible"
I would not complicate explanation. The difference between decrypted in such
'strange'
scenario FS and normal FS is so large, that no fs will work. For example, I
have a BTRFS
on top of -S 4096 encrypted partition. Once on live system (not GRUB) I forgot
to type
-S 4096 and received internal BTRFS error in dmesg saying something about bad
internal
metadata state and then general mount error. So yes, BTRFS can recognize that
this is a
BTRFS partition, but will refuse to mount. In GRUB mode such device (wrong
sector size)
is not recognized too. I think what can be added here is at most a line saying
that FS
may ocassionally detect underlying FS, but will refuse to mount.
> > +
> > +Successfully decrypted disks are named as (cryptoX) and have linear
> > numeration
> > +with other decrypted by cryptodisk devices. If the encrypted disk hosts
> > some higher
> > +level abstraction (like LVM2 or MDRAID) it will be created under a
> > separate device
> > +namespace in addition to the cryptodisk namespace.
>
>
> This is standard cryptomount behavior. I think it makes more sense to
> have some version of this paragrap
