Commit Graph

446 Commits

Author SHA1 Message Date
Glenn Washburn
ec46685ed4 luks2: Use grub_log2ull() to calculate log_sector_size and improve readability
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-18 23:15:05 +01:00
Glenn Washburn
9c3149a2f2 luks2: Better error handling when setting up the cryptodisk
Do some sanity checking on data coming from the LUKS2 header. If segment.size
is "dynamic", verify that the offset is not past the end of disk. Otherwise,
check for errors from grub_strtoull() when converting segment size from
string. If a GRUB_ERR_BAD_NUMBER error was returned, then the string was
not a valid parsable number, so skip the key. If GRUB_ERR_OUT_OF_RANGE was
returned, then there was an overflow in converting to a 64-bit unsigned
integer. So this could be a very large disk (perhaps large RAID array).
In this case skip the key too. Additionally, enforce some other limits
and fail if needed.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-18 23:00:28 +01:00
Glenn Washburn
278201bc31 luks2: Do not handle disks of size GRUB_DISK_SIZE_UNKNOWN for now
Check to make sure that source disk has a known size. If not, print
a message and return error. There are 4 cases where GRUB_DISK_SIZE_UNKNOWN
is set (biosdisk, obdisk, ofdisk, and uboot), and in all those cases
processing continues. So this is probably a bit conservative. However,
3 of the cases seem pathological, and the other, biosdisk, happens when
booting from a CD-ROM. Since I doubt booting from a LUKS2 volume on
a CD-ROM is a big use case, we'll error until someone complains.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-18 15:19:40 +01:00
Glenn Washburn
90fb18632e luks2: Convert to crypt sectors from GRUB native sectors
The function grub_disk_native_sectors(source) returns the number of sectors
of source in GRUB native (512-byte) sectors, not source sized sectors. So
the conversion needs to use GRUB_DISK_SECTOR_BITS, the GRUB native sector
size.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-18 14:49:56 +01:00
Glenn Washburn
b298eed001 luks2: Error check segment.sector_size
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:05 +01:00
Glenn Washburn
cf3a3acff0 cryptodisk: Properly handle non-512 byte sized sectors
By default, dm-crypt internally uses an IV that corresponds to 512-byte
sectors, even when a larger sector size is specified. What this means is
that when using a larger sector size, the IV is incremented every sector.
However, the amount the IV is incremented is the number of 512 byte blocks
in a sector (i.e. 8 for 4K sectors). Confusingly the IV does not correspond
to the number of, for example, 4K sectors. So each 512 byte cipher block in
a sector will be encrypted with the same IV and the IV will be incremented
afterwards by the number of 512 byte cipher blocks in the sector.

There are some encryption utilities which do it the intuitive way and have
the IV equal to the sector number regardless of sector size (ie. the fifth
sector would have an IV of 4 for each cipher block). And this is supported
by dm-crypt with the iv_large_sectors option and also cryptsetup as of 2.3.3
with the --iv-large-sectors, though not with LUKS headers (only with --type
plain). However, support for this has not been included as grub does not
support plain devices right now.

One gotcha here is that the encrypted split keys are encrypted with a hard-
coded 512-byte sector size. So even if your data is encrypted with 4K sector
sizes, the split key encrypted area must be decrypted with a block size of
512 (ie the IV increments every 512 bytes). This made these changes less
aesthetically pleasing than desired.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:05 +01:00
Glenn Washburn
7ed69b1d1c luks2: grub_cryptodisk_t->total_sectors is the max number of device native sectors
We need to convert the sectors from the size of the underlying device to the
cryptodisk sector size; segment.size is in bytes which need to be converted
to cryptodisk sectors as well.

Also, removed an empty statement.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:04 +01:00
Glenn Washburn
b34cb38795 cryptodisk: Add macros GRUB_TYPE_U_MAX/MIN(type) to replace literals
Add GRUB_TYPE_U_MAX/MIN(type) macros to get the max/min values for an
unsigned number with size of type.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:04 +01:00
Glenn Washburn
3e5f7f5112 cryptodisk: Add macro GRUB_TYPE_BITS() to replace some literals
The new macro GRUB_TYPE_BITS(type) returns the number of bits
allocated for type.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:04 +01:00
Glenn Washburn
40abe8576a luks2: Add string "index" to user strings using a json index
This allows error messages to be more easily distinguishable between indexes
and slot keys. The former include the string "index" in the error/debug
string, and the later are surrounded in quotes.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:04 +01:00
Glenn Washburn
2c7933cd9c luks2: Rename json index variables to names that they are obviously json indexes
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:04 +01:00
Glenn Washburn
64691dddb2 luks2: Use more intuitive object name instead of json index in user messages
Use the object name in the json array rather than the 0 based index in the
json array for keyslots, segments, and digests. This is less confusing for
the end user. For example, say you have a LUKS2 device with a key in slot 1
and slot 4. When using the password for slot 4 to unlock the device, the
messages using the index of the keyslot will mention keyslot 1 (its a
zero-based index). Furthermore, with this change the keyslot number will
align with the number used to reference the keyslot when using the
--key-slot argument to cryptsetup.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:04 +01:00
Glenn Washburn
1f006c4c05 luks2: Add idx member to struct grub_luks2_keyslot/segment/digest
This allows code using these structs to know the named key associated with
these json data structures. In the future we can use these to provide better
error messages to the user.

Get rid of idx local variable in luks2_get_keyslot() which was overloaded to
be used for both keyslot and segment slot keys.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:04 +01:00
Glenn Washburn
7904c5ce67 luks2: Make sure all fields of output argument in luks2_parse_digest() are written to
We should assume that the output argument "out" is uninitialized and could
have random data. So, make sure to initialize the segments and keyslots bit
fields because potentially not all bits of those fields are written to.
Otherwise, the digest could say it belongs to keyslots and segments that it
does not.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:04 +01:00
Glenn Washburn
fe902c3f4c luks2: Remove unused argument in grub_error() call
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:04 +01:00
Glenn Washburn
a06549d398 luks2: Convert 8 spaces to tabs
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:04 +01:00
Glenn Washburn
880dfd8f40 disk: Rename grub_disk_get_size() to grub_disk_native_sectors()
The function grub_disk_get_size() is confusingly named because it actually
returns a sector count where the sectors are sized in the GRUB native sector
size. Rename to something more appropriate.

Suggested-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:03 +01:00
Glenn Washburn
85307c34e8 loopback: Do not automaticaly replace existing loopback dev, error instead
If there is a loopback device with the same name as the one to be created,
instead of closing the old one and replacing it with the new one, return an
error instead. If the loopback device was created, its probably being used
by something and just replacing it may cause GRUB to crash unexpectedly.
This fixes obvious problems like "loopback d (d)/somefile". Its not too
onerous to force the user to delete the loopback first with the "-d" switch.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:03 +01:00
Glenn Washburn
6213184b26 luks2: Rename index variable "j" to "i" in luks2_get_keyslot()
Looping variable "j" was named such because the variable name "i" was taken.
Since "i" has been renamed in the previous patch, we can rename "j" to "i".

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-11-20 15:33:41 +01:00
Glenn Washburn
f36193ddf1 luks2: Rename variable "i" to "keyslot_idx" in luks2_get_keyslot()
Variables named "i" are usually looping variables. So, rename it to
"keyslot_idx" to ease luks2_get_keyslot() reading.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-11-20 15:33:41 +01:00
Glenn Washburn
c28907e235 luks2: Use correct index variable when looping in luks2_get_keyslot()
The loop variable "j" should be used to index the digests and segments json
array, instead of the variable "i", which is the keyslot index.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-11-20 15:33:41 +01:00
Glenn Washburn
0eb44d3196 luks2: Rename source disk variable named "disk" to "source" as in luks.c
This makes it more obvious to the reader that the disk referred to is the
source disk, as opposed to say the disk holding the cryptodisk.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-11-20 15:33:41 +01:00
Glenn Washburn
d78ce33e60 cryptodisk: Rename "offset" in grub_cryptodisk_t to "offset_sectors"
This makes it clear that the offset represents sectors, not bytes, in
order to improve readability.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-11-20 15:33:41 +01:00
Glenn Washburn
535998c2e0 cryptodisk: Rename "total_length" field in grub_cryptodisk_t to "total_sectors"
This creates an alignment with grub_disk_t naming of the same field and is
more intuitive as to how it should be used.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-11-20 15:33:41 +01:00
Glenn Washburn
6355ba91e5 cryptodisk: Fix cipher IV mode "plain64" always being set as "plain"
When setting cipher IV mode, detection is done by prefix matching the
cipher IV mode part of the cipher mode string. Since "plain" matches
"plain64", we must check for "plain64" first. Otherwise, "plain64" will
be detected as "plain".

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-10-30 15:37:20 +01:00
Glenn Washburn
84ff10b1c0 cryptodisk: Use cipher name instead of object in error message
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-18 22:31:30 +02:00
Patrick Steinhardt
ee12785f75 luks2: Strip dashes off of the UUID
The UUID header for LUKS2 uses a format with dashes, same as for
LUKS(1). But while we strip these dashes for the latter, we don't for
the former. This isn't wrong per se, but it's definitely inconsistent
for users as they need to use the dashed format for LUKS2 and the
non-dashed format for LUKS when e.g. calling "cryptomount -u $UUID".

Fix this inconsistency by stripping dashes off of the LUKS2 UUID.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-18 22:31:29 +02:00
Glenn Washburn
e1b0992a8d cryptodisk: Fix incorrect calculation of start sector
Here dev is a grub_cryptodisk_t and dev->offset is offset in sectors of size
native to the cryptodisk device. The sector is correctly transformed into
native grub sector size, but then added to dev->offset which is not
transformed. It would be nice if the type system would help us with this.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-11 15:51:04 +02:00
Glenn Washburn
3b3ac0c982 cryptodisk: Unregister cryptomount command when removing module
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-11 15:48:50 +02:00
Patrick Steinhardt
eb77486141 luks2: Improve error reporting when decrypting/verifying key
While we already set up error messages in both luks2_verify_key() and
luks2_decrypt_key(), we do not ever print them. This makes it really
hard to discover why a given key actually failed to decrypt a disk.

Improve this by including the error message in the user-visible output.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-11 15:48:13 +02:00
Patrick Steinhardt
1066336dc8 luks: Fix out-of-bounds copy of UUID
When configuring a LUKS disk, we copy over the UUID from the LUKS header
into the new grub_cryptodisk_t structure via grub_memcpy(). As size
we mistakenly use the size of the grub_cryptodisk_t UUID field, which
is guaranteed to be strictly bigger than the LUKS UUID field we're
copying. As a result, the copy always goes out-of-bounds and copies some
garbage from other surrounding fields. During runtime, this isn't
noticed due to the fact that we always NUL-terminate the UUID and thus
never hit the trailing garbage.

Fix the issue by using the size of the local stripped UUID field.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-11 15:47:39 +02:00
Peter Jones
879c4a8342 lvm: Fix two more potential data-dependent alloc overflows
It appears to be possible to make a (possibly invalid) lvm PV with
a metadata size field that overflows our type when adding it to the
address we've allocated. Even if it doesn't, it may be possible to do so
with the math using the outcome of that as an operand. Check them both.

Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:48 +02:00
Peter Jones
3f05d693d1 malloc: Use overflow checking primitives where we do complex allocations
This attempts to fix the places where we do the following where
arithmetic_expr may include unvalidated data:

  X = grub_malloc(arithmetic_expr);

It accomplishes this by doing the arithmetic ahead of time using grub_add(),
grub_sub(), grub_mul() and testing for overflow before proceeding.

Among other issues, this fixes:
  - allocation of integer overflow in grub_video_bitmap_create()
    reported by Chris Coulson,
  - allocation of integer overflow in grub_png_decode_image_header()
    reported by Chris Coulson,
  - allocation of integer overflow in grub_squash_read_symlink()
    reported by Chris Coulson,
  - allocation of integer overflow in grub_ext2_read_symlink()
    reported by Chris Coulson,
  - allocation of integer overflow in read_section_as_string()
    reported by Chris Coulson.

Fixes: CVE-2020-14309, CVE-2020-14310, CVE-2020-14311

Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:47 +02:00
Peter Jones
f725fa7cb2 calloc: Use calloc() at most places
This modifies most of the places we do some form of:

  X = malloc(Y * Z);

to use calloc(Y, Z) instead.

Among other issues, this fixes:
  - allocation of integer overflow in grub_png_decode_image_header()
    reported by Chris Coulson,
  - allocation of integer overflow in luks_recover_key()
    reported by Chris Coulson,
  - allocation of integer overflow in grub_lvm_detect()
    reported by Chris Coulson.

Fixes: CVE-2020-14308

Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-07-29 16:55:47 +02:00
Patrick Steinhardt
c543d67810 luks2: Propagate error when reading area key fails
When decrypting a given keyslot, all error cases except for one set up
an error and return the error code. The only exception is when we try to
read the area key: instead of setting up an error message, we directly
print it via grub_dprintf().

Convert the outlier to use grub_error() to allow more uniform handling
of errors.

Signed-off-by: Patrick Steinhardt <ps@kps.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-04-21 22:18:26 +02:00
Michael Chang
bdf170d101 mdraid1x_linux: Fix gcc10 error -Werror=array-bounds
We bumped into the build error while testing gcc-10 pre-release.

../../grub-core/disk/mdraid1x_linux.c: In function 'grub_mdraid_detect':
../../grub-core/disk/mdraid1x_linux.c:181:15: error: array subscript <unknown> is outside array bounds of 'grub_uint16_t[0]' {aka 'short unsigned int[0]'} [-Werror=array-bounds]
  181 |      (char *) &sb.dev_roles[grub_le_to_cpu32 (sb.dev_number)]
      |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../grub-core/disk/mdraid1x_linux.c:98:17: note: while referencing 'dev_roles'
   98 |   grub_uint16_t dev_roles[0]; /* Role in array, or 0xffff for a spare, or 0xfffe for faulty.  */
      |                 ^~~~~~~~~
../../grub-core/disk/mdraid1x_linux.c:127:33: note: defined here 'sb'
  127 |       struct grub_raid_super_1x sb;
      |                                 ^~
cc1: all warnings being treated as errors

Apparently gcc issues the warning when trying to access sb.dev_roles
array's member, since it is a zero length array as the last element of
struct grub_raid_super_1x that is allocated sparsely without extra
chunks for the trailing bits, so the warning looks legitimate in this
regard.

As the whole thing here is doing offset computation, it is undue to use
syntax that would imply array member access then take address from it
later. Instead we could accomplish the same thing through basic array
pointer arithmetic to pacify the warning.

Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-03-31 12:17:02 +02:00
Michael Chang
0454b04453 lvm: Add LVM cache logical volume handling
The LVM cache logical volume is the logical volume consisting of the original
and the cache pool logical volume. The original is usually on a larger and
slower storage device while the cache pool is on a smaller and faster one. The
performance of the original volume can be improved by storing the frequently
used data on the cache pool to utilize the greater performance of faster
device.

The default cache mode "writethrough" ensures that any data written will be
stored both in the cache and on the origin LV, therefore grub can be straight
to read the original lv as no data loss is guarenteed.

The second cache mode is "writeback", which delays writing from the cache pool
back to the origin LV to have increased performance. The drawback is potential
data loss if losing the associated cache device.

During the boot time grub reads the LVM offline i.e. LVM volumes are not
activated and mounted, hence it should be fine to read directly from original
lv since all cached data should have been flushed back in the process of taking
it offline.

It is also not much helpful to the situation by adding fsync calls to the
install code. The fsync did not force to write back dirty cache to the original
device and rather it would update associated cache metadata to complete the
write transaction with the cache device. IOW the writes to cached blocks still
go only to the cache device.

To write back dirty cache, as LVM cache did not support dirty cache flush per
block range, there'no way to do it for file. On the other hand the "cleaner"
policy is implemented and can be used to write back "all" dirty blocks in a
cache, which effectively drain all dirty cache gradually to attain and last in
the "clean" state, which can be useful for shrinking or decommissioning a
cache. The result and effect is not what we are looking for here.

In conclusion, as it seems no way to enforce file writes to the original
device, grub may suffer from power failure as it cannot assemble the cache
device and read the dirty data from it. However since the case is only
applicable to writeback mode which is sensitive to data lost in nature, I'd
still like to propose my (relatively simple) patch and treat reading dirty
cache as improvement.

Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-03-31 11:59:35 +02:00
Peter Jones
d5a32255de misc: Make grub_strtol() "end" pointers have safer const qualifiers
Currently the string functions grub_strtol(), grub_strtoul(), and
grub_strtoull() don't declare the "end" pointer in such a way as to
require the pointer itself or the character array to be immutable to the
implementation, nor does the C standard do so in its similar functions,
though it does require us not to change any of it.

The typical declarations of these functions follow this pattern:

long
strtol(const char * restrict nptr, char ** restrict endptr, int base);

Much of the reason for this is historic, and a discussion of that
follows below, after the explanation of this change.  (GRUB currently
does not include the "restrict" qualifiers, and we name the arguments a
bit differently.)

The implementation is semantically required to treat the character array
as immutable, but such accidental modifications aren't stopped by the
compiler, and the semantics for both the callers and the implementation
of these functions are sometimes also helped by adding that requirement.

This patch changes these declarations to follow this pattern instead:

long
strtol(const char * restrict nptr,
       const char ** const restrict endptr,
       int base);

This means that if any modification to these functions accidentally
introduces either an errant modification to the underlying character
array, or an accidental assignment to endptr rather than *endptr, the
compiler should generate an error.  (The two uses of "restrict" in this
case basically mean strtol() isn't allowed to modify the character array
by going through *endptr, and endptr isn't allowed to point inside the
array.)

It also means the typical use case changes to:

  char *s = ...;
  const char *end;
  long l;

  l = strtol(s, &end, 10);

Or even:

  const char *p = str;
  while (p && *p) {
	  long l = strtol(p, &p, 10);
	  ...
  }

This fixes 26 places where we discard our attempts at treating the data
safely by doing:

  const char *p = str;
  long l;

  l = strtol(p, (char **)&ptr, 10);

It also adds 5 places where we do:

  char *p = str;
  while (p && *p) {
	  long l = strtol(p, (const char ** const)&p, 10);
	  ...
	  /* more calls that need p not to be pointer-to-const */
  }

While moderately distasteful, this is a better problem to have.

With one minor exception, I have tested that all of this compiles
without relevant warnings or errors, and that /much/ of it behaves
correctly, with gcc 9 using 'gcc -W -Wall -Wextra'.  The one exception
is the changes in grub-core/osdep/aros/hostdisk.c , which I have no idea
how to build.

Because the C standard defined type-qualifiers in a way that can be
confusing, in the past there's been a slow but fairly regular stream of
churn within our patches, which add and remove the const qualifier in many
of the users of these functions.  This change should help avoid that in
the future, and in order to help ensure this, I've added an explanation
in misc.h so that when someone does get a compiler warning about a type
error, they have the fix at hand.

The reason we don't have "const" in these calls in the standard is
purely anachronistic: C78 (de facto) did not have type qualifiers in the
syntax, and the "const" type qualifier was added for C89 (I think; it
may have been later).  strtol() appears to date from 4.3BSD in 1986,
which means it could not be added to those functions in the standard
without breaking compatibility, which is usually avoided.

The syntax chosen for type qualifiers is what has led to the churn
regarding usage of const, and is especially confusing on string
functions due to the lack of a string type.  Quoting from C99, the
syntax is:

 declarator:
  pointer[opt] direct-declarator
 direct-declarator:
  identifier
  ( declarator )
  direct-declarator [ type-qualifier-list[opt] assignment-expression[opt] ]
  ...
  direct-declarator [ type-qualifier-list[opt] * ]
  ...
 pointer:
  * type-qualifier-list[opt]
  * type-qualifier-list[opt] pointer
 type-qualifier-list:
  type-qualifier
  type-qualifier-list type-qualifier
 ...
 type-qualifier:
  const
  restrict
  volatile

So the examples go like:

const char foo;			// immutable object
const char *foo;		// mutable pointer to object
char * const foo;		// immutable pointer to mutable object
const char * const foo;		// immutable pointer to immutable object
const char const * const foo; 	// XXX extra const keyword in the middle
const char * const * const foo; // immutable pointer to immutable
				//   pointer to immutable object
const char ** const foo;	// immutable pointer to mutable pointer
				//   to immutable object

Making const left-associative for * and right-associative for everything
else may not have been the best choice ever, but here we are, and the
inevitable result is people using trying to use const (as they should!),
putting it at the wrong place, fighting with the compiler for a bit, and
then either removing it or typecasting something in a bad way.  I won't
go into describing restrict, but its syntax has exactly the same issue
as with const.

Anyway, the last example above actually represents the *behavior* that's
required of strtol()-like functions, so that's our choice for the "end"
pointer.

Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-02-28 12:41:29 +01:00
Patrick Steinhardt
9404c41953 luks2: Add missing newline to debug message
The debug message printed when decryption with a keyslot fails is
missing its trailing newline. Add it to avoid mangling it with
subsequent output.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-02-28 12:27:55 +01:00
Patrick Steinhardt
365e0cc3e7 disk: Implement support for LUKS2
With cryptsetup 2.0, a new version of LUKS was introduced that breaks
compatibility with the previous version due to various reasons. GRUB
currently lacks any support for LUKS2, making it impossible to decrypt
disks encrypted with that version. This commit implements support for
this new format.

Note that LUKS1 and LUKS2 are quite different data formats. While they
do share the same disk signature in the first few bytes, representation
of encryption parameters is completely different between both versions.
While the former version one relied on a single binary header, only,
LUKS2 uses the binary header only in order to locate the actual metadata
which is encoded in JSON. Furthermore, the new data format is a lot more
complex to allow for more flexible setups, like e.g. having multiple
encrypted segments and other features that weren't previously possible.
Because of this, it was decided that it doesn't make sense to keep both
LUKS1 and LUKS2 support in the same module and instead to implement it
in two different modules luks and luks2.

The proposed support for LUKS2 is able to make use of the metadata to
decrypt such disks. Note though that in the current version, only the
PBKDF2 key derival function is supported. This can mostly attributed to
the fact that the libgcrypt library currently has no support for either
Argon2i or Argon2id, which are the remaining KDFs supported by LUKS2. It
wouldn't have been much of a problem to bundle those algorithms with
GRUB itself, but it was decided against that in order to keep down the
number of patches required for initial LUKS2 support. Adding it in the
future would be trivial, given that the code structure is already in
place.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-01-10 14:30:24 +01:00
Patrick Steinhardt
dd3f49b106 luks: Move configuration of ciphers into cryptodisk
The luks module contains quite a lot of logic to parse cipher and
cipher-mode strings like aes-xts-plain64 into constants to apply them
to the grub_cryptodisk_t structure. This code will be required by the
upcoming luks2 module, as well, which is why this commit moves it into
its own function grub_cryptodisk_setcipher in the cryptodisk module.
While the strings are probably rather specific to the LUKS modules, it
certainly does make sense that the cryptodisk module houses code to set
up its own internal ciphers instead of hosting that code in the luks
module.

Except for necessary adjustments around error handling, this commit does
an exact move of the cipher configuration logic from luks.c to
cryptodisk.c. Any behavior changes are unintentional.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-01-10 14:29:37 +01:00
Patrick Steinhardt
5324c335b1 afsplitter: Move into its own module
While the AFSplitter code is currently used only by the luks module,
upcoming support for luks2 will add a second module that depends on it.
To avoid any linker errors when adding the code to both modules because
of duplicated symbols, this commit moves it into its own standalone
module afsplitter as a preparatory step.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-01-10 14:27:49 +01:00
Eric Snowberg
ffe3921538 efidisk: NULL pointer dereference in grub_efidisk_get_device_name()
Function grub_efi_find_last_device_path() may return NULL when called
from grub_efidisk_get_device_name().

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2019-04-02 13:09:08 +02:00
Eric Snowberg
4fff586386 efidisk: NULL pointer dereference in is_child()
Function grub_efi_find_last_device() path may return NULL when called
from is_child().

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2019-04-02 13:08:22 +02:00
Eric Snowberg
3c65959673 efidisk: Write to NULL pointer ldp
Function grub_efi_find_last_device_path() may return constant NULL when
called from find_parent_device().

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2019-04-02 13:07:14 +02:00
Vladimir Serbinenko
384091967d Rename grub_disk members
Otherwise it horribly clashes with gnulib when it's
replacing open/write/read/close

Signed-off-by: Vladimir Serbinenko <phcoder@google.com>
2019-03-25 15:14:52 +01:00
Eric Snowberg
3434ddec0e ieee1275: obdisk driver
Add a new disk driver called obdisk for IEEE1275 platforms.  Currently
the only platform using this disk driver is SPARC, however other IEEE1275
platforms could start using it if they so choose.  While the functionality
within the current IEEE1275 ofdisk driver may be suitable for PPC and x86, it
presented too many problems on SPARC hardware.

Within the old ofdisk, there is not a way to determine the true canonical
name for the disk.  Within Open Boot, the same disk can have multiple names
but all reference the same disk.  For example the same disk can be referenced
by its SAS WWN, using this form:

/pci@302/pci@2/pci@0/pci@17/LSI,sas@0/disk@w5000cca02f037d6d,0

It can also be referenced by its PHY identifier using this form:

/pci@302/pci@2/pci@0/pci@17/LSI,sas@0/disk@p0

It can also be referenced by its Target identifier using this form:

/pci@302/pci@2/pci@0/pci@17/LSI,sas@0/disk@0

Also, when the LUN=0, it is legal to omit the ,0 from the device name.  So with
the disk above, before taking into account the device aliases, there are 6 ways
to reference the same disk.

Then it is possible to have 0 .. n device aliases all representing the same disk.
Within this new driver the true canonical name is determined using the the
IEEE1275 encode-unit and decode-unit commands when address_cells == 4.  This
will determine the true single canonical name for the device so multiple ihandles
are not opened for the same device.  This is what frequently happens with the old
ofdisk driver.  With some devices when they are opened multiple times it causes
the entire system to hang.

Another problem solved with this driver is devices that do not have a device
alias can be booted and used within GRUB. Within the old ofdisk, this was not
possible, unless it was the original boot device.  All devices behind a SAS
or SCSI parent can be found.   Within the old ofdisk, finding these disks
relied on there being an alias defined.  The alias requirement is not
necessary with this new driver.  It can also find devices behind a parent
after they have been hot-plugged.  This is something that is not possible
with the old ofdisk driver.

The old ofdisk driver also incorrectly assumes that the device pointing to by a
device alias is in its true canonical form. This assumption is never made with
this new driver.

Another issue solved with this driver is that it properly caches the ihandle
for all open devices.  The old ofdisk tries to do this by caching the last
opened ihandle.  However this does not work properly because the layer above
does not use a consistent device name for the same disk when calling into the
driver.  This is because the upper layer uses the bootpath value returned within
/chosen, other times it uses the device alias, and other times it uses the
value within grub.cfg.  It does not have a way to figure out that these devices
are the same disk.  This is not a problem with this new driver.

Due to the way GRUB repeatedly opens and closes the same disk. Caching the
ihandle is important on SPARC.  Without caching, some SAS devices can take
15 - 20 minutes to get to the GRUB menu. This ihandle caching is not possible
without correctly having the canonical disk name.

When available, this driver also tries to use the deblocker #blocks and
a way of determining the disk size.

Finally and probably most importantly, this new driver is also capable of
seeing all partitions on a GPT disk.  With the old driver, the GPT
partition table can not be read and only the first partition on the disk
can be seen.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2019-03-12 20:04:07 +01:00
Cristian Ciocaltea
088f7f56e5 uboot: Add the missing disk write operation support
uboot_disk_write() is currently lacking the write support
to storage devices because, historically, those devices did not
implement block_write() in U-Boot.

The solution has been tested using a patched U-Boot loading
and booting GRUB in a QEMU vexpress-a9 environment.
The disk write operations were triggered with GRUB's save_env
command.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2019-01-22 15:23:51 +01:00
Vladimir Serbinenko
ca0a4f689a verifiers: File type for fine-grained signature-verification controlling
Let's provide file type info to the I/O layer. This way verifiers
framework and its users will be able to differentiate files and verify
only required ones.

This is preparatory patch.

Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com>
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Ross Philipson <ross.philipson@oracle.com>
2018-11-09 13:25:31 +01:00
Goffredo Baroncelli
fa926cb4b4 btrfs: Make more generic the code for RAID 6 rebuilding
The original code which handles the recovery of a RAID 6 disks array
assumes that all reads are multiple of 1 << GRUB_DISK_SECTOR_BITS and it
assumes that all the I/O is done via the struct grub_diskfilter_segment.
This is not true for the btrfs code. In order to reuse the native
grub_raid6_recover() code, it is modified to not call
grub_diskfilter_read_node() directly, but to call an handler passed
as an argument.

Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2018-10-31 12:07:29 +01:00