Add the options --key-file, --keyfile-offset, and --keyfile-size to
cryptomount and code to put read the requested key file data and pass
via the cargs struct. Note, key file data is for all intents and purposes
equivalent to a password given to cryptomount. So there is no need to
enable support for key files in the various crypto backends (e.g. LUKS1)
because the key data is passed just as if it were a password.
Signed-off-by: John Lane <john@lane.uk.net>
Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The GRUB is failing to build with GCC-12 in many places like this:
In function 'init_cbfsdisk',
inlined from 'grub_mod_init' at ../../grub-core/fs/cbfs.c:391:3:
../../grub-core/fs/cbfs.c:345:7: error: array subscript 0 is outside array bounds of 'grub_uint32_t[0]' {aka 'unsigned int[]'} [-Werror=array-bounds]
345 | ptr = *(grub_uint32_t *) 0xfffffffc;
| ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This is caused by GCC regression in 11/12 [1]. In a nut shell, the
warning is about detected invalid accesses at non-zero offsets to NULL
pointers. Since hardwired constant address is treated as NULL plus an
offset in the same underlying code, the warning is therefore triggered.
Instead of inserting #pragma all over the places where literal pointers
are accessed to avoid diagnosing array-bounds, we can try to borrow the
idea from Linux kernel that the absolute_pointer() macro [2][3] is used
to disconnect a pointer using literal address from it's original object,
hence GCC won't be able to make assumptions on the boundary while doing
pointer arithmetic. With that we can greatly reduce the code we have to
cover up by making initial literal pointer assignment to use the new
wrapper but not having to track everywhere literal pointers are
accessed. This also makes code looks cleaner.
Please note the grub_absolute_pointer() macro requires to be invoked in
a function as long as it is compound expression. Some global variables
with literal pointers has been changed to local ones in order to use
grub_absolute_pointer() to initialize it. The shuffling is basically done
in a selective and careful way that the variable's scope doesn't matter
being local or global, for example, the global variable must not get
modified at run time throughout. For the record, here's the list of
global variables got shuffled in this patch:
grub-core/commands/i386/pc/drivemap.c:int13slot
grub-core/term/i386/pc/console.c:bios_data_area
grub-core/term/ns8250.c:serial_hw_io_addr
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578
[2] https://elixir.bootlin.com/linux/v5.16.14/source/include/linux/compiler.h#L180
[3] https://elixir.bootlin.com/linux/v5.16.14/source/include/linux/compiler-gcc.h#L31
Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
- Fix type of size variable in luks2_verify_key()
- Avoid redefinition of SIZE_MAX and ATTRIBUTE_ERROR
- Work around gnulib's int types on older compilers
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
gnulib defines go in config-util.h, and we need to know whether to
provide duplicates in config.h or not.
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The Coverity output is:
*** CID 366905: Memory - illegal accesses (USE_AFTER_FREE)
/grub-core/disk/cryptodisk.c: 1064 in grub_cryptodisk_scan_device_real()
1058 cleanup:
1059 if (askpass)
1060 {
1061 cargs->key_len = 0;
1062 grub_free (cargs->key_data);
1063 }
>>> CID 366905: Memory - illegal accesses (USE_AFTER_FREE)
>>> Using freed pointer "dev".
1064 return dev;
1065 }
1066
1067 #ifdef GRUB_UTIL
1068 #include <grub/util/misc.h>
1069 grub_err_t
Here the "dev" variable can point to a freed cryptodisk device if the
function grub_cryptodisk_insert() fails. This can happen only on a OOM
condition, but when this happens grub_cryptodisk_insert() calls grub_free on
the passed device. Since grub_cryptodisk_scan_device_real() assumes that
grub_cryptodisk_insert() is always successful, it will return the device,
though the device was freed.
Change grub_cryptodisk_insert() to not free the passed device on failure.
Then on grub_cryptodisk_insert() failure, free the device pointer. This is
done by going to the label "error", which will call cryptodisk_close() to
free the device and set the device pointer to NULL, so that a pointer to
freed memory is not returned.
Fixes: CID 366905
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Call grub_partition_get_name() unconditionally to initialize the part
variable. Then part will only be NULL when grub_partition_get_name() errors.
Note that when source->partition is NULL, then grub_partition_get_name()
returns an allocated empty string. So no comma or partition will be printed,
as desired.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Note that cargs.search_uuid does not need to be initialized in various parts
of the cryptomount argument parsing, just once when cargs is declared with
a struct initializer. The previous code used a global variable which would
retain the value across cryptomount invocations.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The crypto device modules should only be setting up the crypto devices and
not getting user input. This has the added benefit of simplifying the code
such that three essentially duplicate pieces of code are merged into one.
Add documentation of passphrase option for cryptomount as it is now usable.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Previously, the cryptomount arguments were passed by global variable and
function call argument, neither of which are ideal. This change passes data
via a grub_cryptomount_args struct, which can be added to over time as
opposed to continually adding arguments to the cryptodisk scan and
recover_key.
As an example, passing a password as a cryptomount argument is implemented.
However, the backends are not implemented, so testing this will return a not
implemented error.
Also, add comments to cryptomount argument parsing to make it more obvious
which argument states are being handled.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
When a cryptmount is specified with a UUID, but no cryptodisk backends find
a disk with that UUID, return a more detailed message giving telling the
user that they might not have a needed cryptobackend module loaded.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Update such that "cryptomount -u UUID" will not print two error messages
when an invalid passphrase is given and the most relevant error message
will be displayed.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This displays an error notifying the user that they'll want to load
a backend module to make cryptomount useful.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The global "have_it" was never used by the crypto-backends, but was used to
determine if a crypto-backend successfully mounted a cryptodisk with a given
UUID. This is not needed however, because grub_device_iterate() will return
1 if and only if grub_cryptodisk_scan_device() returns 1. And
grub_cryptodisk_scan_device() will now only return 1 if a search_uuid has
been specified and a cryptodisk was successfully setup by a crypto-backend or
a cryptodisk of the requested UUID is already open.
To implement this grub_cryptodisk_scan_device_real() is modified to return
a cryptodisk or NULL on failure and having the appropriate grub_errno set to
indicated failure. Note that grub_cryptodisk_scan_device_real() will fail now
with a new errno GRUB_ERR_BAD_MODULE when none of the cryptodisk backend
modules succeed in identifying the source disk.
With this change grub_device_iterate() will return 1 when a crypto device is
successfully decrypted or when the source device has already been successfully
opened. Prior to this change, trying to mount an already successfully opened
device would trigger an error with the message "no such cryptodisk found",
which is at best misleading. The mount should silently succeed in this case,
which is what happens with this patch.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Commit 23e39f50ca7a (disk/ldm: Make sure comp data is freed before exiting from
make_vg()) fixed several spots in make_vg() where comp data was leaking memory
when an error was being handled but missed one. To avoid leaking memory, comp
should be freed when an error is being handled after comp has been successfully
allocated memory in the for loop.
Fixes: 23e39f50ca7a (disk/ldm: Make sure comp data is freed before exiting from make_vg())
Fixes: CID 73804
Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Currently the grub_diskfilter_memberlist() function returns all physical
volumes added to a volume group to which a logical volume (LV) belongs.
However, this is suboptimal as it doesn't fit the intended behavior of
returning underlying devices that make up the LV. To give a clear
picture, the result should be identical to running commands below to
display the logical volumes with underlying physical volumes in use.
localhost:~ # lvs -o lv_name,vg_name,devices /dev/system/root
LV VG Devices
root system /dev/vda2(512)
localhost:~ # lvdisplay --maps /dev/system/root
--- Logical volume ---
...
--- Segments ---
Logical extents 0 to 4604:
Type linear
Physical volume /dev/vda2
Physical extents 512 to 5116
As shown above, we can know system-root LV uses only /dev/vda2 to
allocate it's extents, or we can say that /dev/vda2 is the member device
comprising the system-root LV.
It is important to be precise on the member devices, because that helps
to avoid pulling in excessive dependency. Let's use an example to
demonstrate why it is needed.
localhost:~ # findmnt /
TARGET SOURCE FSTYPE OPTIONS
/ /dev/mapper/system-root ext4 rw,relatime
localhost:~ # pvs
PV VG Fmt Attr PSize PFree
/dev/mapper/data system lvm2 a-- 1020.00m 0
/dev/vda2 system lvm2 a-- 19.99g 0
localhost:~ # cryptsetup status /dev/mapper/data
/dev/mapper/data is active and is in use.
type: LUKS1
cipher: aes-xts-plain64
keysize: 512 bits
key location: dm-crypt
device: /dev/vdb
sector size: 512
offset: 4096 sectors
size: 2093056 sectors
mode: read/write
localhost:~ # vgs
VG #PV #LV #SN Attr VSize VFree
system 2 3 0 wz--n- 20.98g 0
localhost:~ # lvs -o lv_name,vg_name,devices
LV VG Devices
data system /dev/mapper/data(0)
root system /dev/vda2(512)
swap system /dev/vda2(0)
We can learn from above that /dev/mapper/data is an encrypted volume and
also gets assigned to volume group "system" as one of it's physical
volumes. And also it is not used by root device, /dev/mapper/system-root,
for allocating extents, so it shouldn't be taking part in the process of
setting up GRUB to access root device.
However, running grub-install reports error as volume group "system"
contains encrypted volume.
error: attempt to install to encrypted disk without cryptodisk
enabled. Set `GRUB_ENABLE_CRYPTODISK=y' in file `/etc/default/grub'.
Certainly we can enable GRUB_ENABLE_CRYPTODISK=y and move on, but that
is not always acceptable since the server may need to be booted unattended.
Additionally, typing passphrase for every system startup can be a big
hassle of which most users would like to avoid.
This patch solves the problem by returning exact physical volume, /dev/vda2,
rightly used by system-root from the example above, thus grub-install will
not error out because the excessive encrypted device to boot the root device
is not configured.
Signed-off-by: Michael Chang <mchang@suse.com>
Tested-by: Olav Reinert <seroton10@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The GNU gettext only supports the ISO C99 macros for integral
types. If there is a need to use unsupported formatting macros,
e.g. PRIuGRUB_UINT64_T, according to [1] the number to a string
conversion should be separated from the code printing message
requiring the internationalization. So, the function grub_snprintf()
is used to print the numeric values to an intermediate buffer and
the internationalized message contains a string format directive.
[1] https://www.gnu.org/software/gettext/manual/html_node/Preparing-Strings.html#No-string-concatenation
Signed-off-by: Miguel Ángel Arruga Vivas <rosen644835@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The grub_error() has a format string expecting two arguments, but only one
provided. According to the comments in the struct grub_nv_super definition,
the version field looks like a version number where major.minor is encoded
as each a byte in the two-byte short.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This error message comes from the grub_print_error() in
grub_pata_device_initialize(), which does not pass on the error, and is
raised in check_device(). The function check_device() needs to return this
as an error because check_device() is also used in grub_pata_open(), which
does pass on this error to indicate that the device can not be used.
This is actually not an error when displayed by grub_pata_device_initialize()
because it just indicates that there are no pata devices seen. This may be
confusing to end users who do not have pata devices yet are loading the
pata module (perhaps implicitly via nativedisk). This also causes unnecessary
output which may need to be accounted for in functional testing.
Instead print to the debug log when check_device() raises this "error" and
pop the error from the error stack. If there is another error on the stack
then print the error stack as those should be real errors.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Acked-by: Paul Menzel <pmenzel@molgen.mpg.de>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This prevents infinite recursion in the diskfilter verification code.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
rlocn->offset is read directly from disk and added to the metadatabuf
pointer to create a pointer to a block of metadata. It's a 64-bit
quantity so as long as you don't overflow you can set subsequent
pointers to point anywhere in memory.
Require that rlocn->offset fits within the metadata buffer size.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
We could reach the end of valid metadata and not realize, leading to
some buffer overreads. Check if we have reached the end and bail.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Clean up a bunch of cases where we could have strstr() fail and lead to
us dereferencing NULL.
We'll still leak memory in some cases (loops don't clean up allocations
from earlier iterations if a later iteration fails) but at least we're
not crashing.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
There's an if block for the presence of "physical_volumes {", but if
that block is absent, then p remains NULL and a NULL-deref will result
when looking for logical volumes.
It doesn't seem like LVM makes sense without physical volumes, so error
out rather than crashing.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This catches at least some OOB reads, and it's possible I suppose that
if 2 * mda_size is less than GRUB_LVM_MDA_HEADER_SIZE it might catch some
OOB writes too (although that hasn't showed up as a crash in fuzzing yet).
It's a bit ugly and I'd appreciate better suggestions.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
We unconditionally trusted offset_xl from the LVM label header, even if
it told us that the PV header/disk locations were way off past the end
of the data we read from disk.
Require that the offset be sane, fixing an OOB read and crash.
Fixes: CID 314367, CID 314371
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The encrypt and decrypt functions expect a grub_size_t. So, we need to
ensure that the constant bit shift is using grub_size_t rather than
unsigned int when it is performing the shift.
Fixes: CID 307788
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The problem here is that the memory allocated to the variable lv is not
yet inserted into the list that is being processed at the label fail2.
As we can already see at line 342, which correctly frees lv before going
to fail2, we should also be doing that at these earlier jumps to fail2.
Fixes: CID 73824
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Several error handling paths in make_vg() do not free comp data before
jumping to fail2 label and returning from the function. This will leak
memory. So, let's fix all issues of that kind.
Fixes: CID 73804
Signed-off-by: Marco A Benatto <mbenatto@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
