[PATCH] Add some randomness to TCP source port selection.
GRUB uses a static source TCP port and increments for each new connection. When rapidly restarting GRUB this can cause issues with some firewalls that suspect that a reply attack is happening. In addition GRUB does not ACK the last FIN,ACK when booting the kernel and initrd from HTTP for example. This cause the remote HTTP server to keep the TCP session in TIME_WAIT and reject new connections from the same port combination when restarted quickly. This helps to work around both problems by shifting the source port by a small amount based on time. The missing final ACK should also be addressed, but I'm not sure how to resolve that. Signed-off-by: Robert LeBlanc --- grub-core/net/tcp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/grub-core/net/tcp.c b/grub-core/net/tcp.c index 93dee0caa..2eefd3168 100644 --- a/grub-core/net/tcp.c +++ b/grub-core/net/tcp.c @@ -569,7 +569,7 @@ grub_net_tcp_open (char *server, struct grub_net_network_level_interface *inf; grub_net_network_level_address_t gateway; grub_net_tcp_socket_t socket; - static grub_uint16_t in_port = 21550; + grub_uint16_t in_port = 21550 + grub_get_time_ms () % 256; struct grub_net_buff *nb; struct tcphdr *tcph; int i; @@ -603,7 +603,7 @@ grub_net_tcp_open (char *server, socket->inf = inf; socket->out_nla = addr; socket->ll_target_addr = ll_target_addr; - socket->in_port = in_port++; + socket->in_port = in_port; socket->recv_hook = recv_hook; socket->error_hook = error_hook; socket->fin_hook = fin_hook; -- 2.35.1 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 1/3] grub-mkrescue: Add support for FAT and NTFS on EFI boot
In order to add file system transposition support for UEFI, i.e. the ability
to copy the content of an ISO9660 grub-mkrescue ISO image onto user-formatted
media, and have that boot on UEFI systems, the first thing we need to do is
add support for the file systems that are natively handled by UEFI.
This mandatorily includes FAT and we also include NTFS as the latter is also
commonly supported on modern x64 platforms.
Signed-off-by: Pete Batard
---
util/grub-mkrescue.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/util/grub-mkrescue.c b/util/grub-mkrescue.c
index ba89b1394..1257476fb 100644
--- a/util/grub-mkrescue.c
+++ b/util/grub-mkrescue.c
@@ -754,6 +754,9 @@ main (int argc, char *argv[])
grub_install_push_module ("part_gpt");
grub_install_push_module ("part_msdos");
+ grub_install_push_module ("fat");
+ /* Many modern UEFI systems also have native support for NTFS */
+ grub_install_push_module ("ntfs");
imgname = grub_util_path_concat (2, efidir_efi_boot, "bootia64.efi");
make_image_fwdisk_abs (GRUB_INSTALL_PLATFORM_IA64_EFI, "ia64-efi",
imgname);
@@ -831,6 +834,8 @@ main (int argc, char *argv[])
free (efidir);
grub_install_pop_module ();
grub_install_pop_module ();
+ grub_install_pop_module ();
+ grub_install_pop_module ();
}
grub_install_push_module ("part_apple");
--
2.36.0.windows.1
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 2/3] grub-mkrescue: Preserve a copy of the EFI bootloaders on the ISO9660 file system
To enable file system transposition support for UEFI, we also must ensure that
there exists a copy of the EFI bootloaders, that are currently embedded in the
efi.img for xorriso, at their expected UEFI location on the ISO9660 file system.
This is accomplished by removing the use of a temporary directory to create the
efi/ content, to instead place it at the root of the ISO9660 content.
Signed-off-by: Pete Batard
---
util/grub-mkrescue.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/util/grub-mkrescue.c b/util/grub-mkrescue.c
index 1257476fb..4596f0ea7 100644
--- a/util/grub-mkrescue.c
+++ b/util/grub-mkrescue.c
@@ -745,9 +745,8 @@ main (int argc, char *argv[])
|| source_dirs[GRUB_INSTALL_PLATFORM_RISCV32_EFI]
|| source_dirs[GRUB_INSTALL_PLATFORM_RISCV64_EFI])
{
- char *efidir = grub_util_make_temporary_dir ();
- char *efidir_efi = grub_util_path_concat (2, efidir, "efi");
- char *efidir_efi_boot = grub_util_path_concat (3, efidir, "efi", "boot");
+ char *efidir_efi = grub_util_path_concat (2, iso9660_dir, "efi");
+ char *efidir_efi_boot = grub_util_path_concat (3, iso9660_dir, "efi",
"boot");
char *imgname, *img32, *img64, *img_mac = NULL;
char *efiimgfat;
grub_install_mkdir_p (efidir_efi_boot);
@@ -828,10 +827,9 @@ main (int argc, char *argv[])
xorriso_push ("-efi-boot-part");
xorriso_push ("--efi-boot-image");
- grub_util_unlink_recursive (efidir);
+ /* Don't unlink the efidir_efi_boot directory so that we have a
duplicate on the ISO9660 file system. */
free (efiimgfat);
free (efidir_efi);
- free (efidir);
grub_install_pop_module ();
grub_install_pop_module ();
grub_install_pop_module ();
--
2.36.0.windows.1
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 3/3] grub-mkrescue: Search by file UUID file rather than partition UUID for EFI boot
The final piece we need to add file system transposition support for UEFI
is to ensure that we can locate the boot media regardless of how the boot
partition was instantiated. Especially, we do not want to be reliant on
brittle partition UUIDs, as these only work if a boot media is duplicated at
the block level and not at the file system level.
To accomplish this for EFI boot, we now create a UUID file in a .disk/
directory, that we can then search for.
Signed-off-by: Pete Batard
---
util/grub-mkrescue.c | 47
1 file changed, 39 insertions(+), 8 deletions(-)
diff --git a/util/grub-mkrescue.c b/util/grub-mkrescue.c
index 4596f0ea7..b7a387d8b 100644
--- a/util/grub-mkrescue.c
+++ b/util/grub-mkrescue.c
@@ -261,7 +261,26 @@ make_image_abs (enum grub_install_plat plat,
load_cfg = grub_util_make_temporary_file ();
load_cfg_f = grub_util_fopen (load_cfg, "wb");
- fprintf (load_cfg_f, "search --fs-uuid --set=root %s\n", iso_uuid);
+ /*
+ * A UEFI bootable media should support file system transposition (e.g.
extracting
+ * an ISO9660 content to a FAT32 media that was formatted by the user).
Therefore,
+ * for EFI platforms, we search for a specific UUID file rather than a
partition UUID.
+ */
+ switch (plat)
+{
+ case GRUB_INSTALL_PLATFORM_I386_EFI:
+ case GRUB_INSTALL_PLATFORM_X86_64_EFI:
+ case GRUB_INSTALL_PLATFORM_IA64_EFI:
+ case GRUB_INSTALL_PLATFORM_ARM_EFI:
+ case GRUB_INSTALL_PLATFORM_ARM64_EFI:
+ case GRUB_INSTALL_PLATFORM_RISCV32_EFI:
+ case GRUB_INSTALL_PLATFORM_RISCV64_EFI:
+ fprintf (load_cfg_f, "search --set=root --file /.disk/%s.uuid\n",
iso_uuid);
+ break;
+ default:
+ fprintf (load_cfg_f, "search --fs-uuid --set=root %s\n", iso_uuid);
+ break;
+}
fprintf (load_cfg_f, "set prefix=(${root})/boot/grub\n");
write_part (load_cfg_f, source_dirs[plat]);
@@ -745,10 +764,11 @@ main (int argc, char *argv[])
|| source_dirs[GRUB_INSTALL_PLATFORM_RISCV32_EFI]
|| source_dirs[GRUB_INSTALL_PLATFORM_RISCV64_EFI])
{
+ FILE *f;
char *efidir_efi = grub_util_path_concat (2, iso9660_dir, "efi");
char *efidir_efi_boot = grub_util_path_concat (3, iso9660_dir, "efi",
"boot");
char *imgname, *img32, *img64, *img_mac = NULL;
- char *efiimgfat;
+ char *efiimgfat, *iso_uuid_file, *diskdir, *diskdir_uuid;
grub_install_mkdir_p (efidir_efi_boot);
grub_install_push_module ("part_gpt");
@@ -757,36 +777,47 @@ main (int argc, char *argv[])
/* Many modern UEFI systems also have native support for NTFS */
grub_install_push_module ("ntfs");
+ /* Create a '.disk/.uuid' file that can be used to
locate the boot media. */
+ diskdir = grub_util_path_concat (2, iso9660_dir, ".disk");
+ grub_install_mkdir_p (diskdir);
+ iso_uuid_file = xasprintf ("%s.uuid", iso_uuid);
+ diskdir_uuid = grub_util_path_concat (2, diskdir, iso_uuid_file);
+ f = grub_util_fopen (diskdir_uuid, "wb");
+ fclose (f);
+ free (iso_uuid_file);
+ free (diskdir_uuid);
+ free (diskdir);
+
imgname = grub_util_path_concat (2, efidir_efi_boot, "bootia64.efi");
make_image_fwdisk_abs (GRUB_INSTALL_PLATFORM_IA64_EFI, "ia64-efi",
imgname);
free (imgname);
grub_install_push_module ("part_apple");
img64 = grub_util_path_concat (2, efidir_efi_boot, "bootx64.efi");
- make_image_fwdisk_abs (GRUB_INSTALL_PLATFORM_X86_64_EFI, "x86_64-efi",
img64);
+ make_image_abs (GRUB_INSTALL_PLATFORM_X86_64_EFI, "x86_64-efi", img64);
grub_install_pop_module ();
grub_install_push_module ("part_apple");
img32 = grub_util_path_concat (2, efidir_efi_boot, "bootia32.efi");
- make_image_fwdisk_abs (GRUB_INSTALL_PLATFORM_I386_EFI, "i386-efi",
img32);
+ make_image_abs (GRUB_INSTALL_PLATFORM_I386_EFI, "i386-efi", img32);
grub_install_pop_module ();
imgname = grub_util_path_concat (2, efidir_efi_boot, "bootarm.efi");
- make_image_fwdisk_abs (GRUB_INSTALL_PLATFORM_ARM_EFI, "arm-efi",
imgname);
+ make_image_abs (GRUB_INSTALL_PLATFORM_ARM_EFI, "arm-efi", imgname);
free (imgname);
imgname = grub_util_path_concat (2, efidir_efi_boot, "bootaa64.efi");
- make_image_fwdisk_abs (GRUB_INSTALL_PLATFORM_ARM64_EFI, "arm64-efi",
+ make_image_abs (GRUB_INSTALL_PLATFORM_ARM64_EFI, "arm64-efi",
imgname);
free (imgname);
imgname = grub_util_path_concat (2, efidir_efi_boot, "bootriscv32.efi");
- make_image_fwdisk_abs (GRUB_INSTALL_PLATFORM_RISCV32_EFI, "riscv32-efi",
+ make_image_abs (GRUB_INSTALL_PLATFORM_RISCV32_EFI, "riscv32-efi",
imgname);
free (imgname);
imgname = grub_util_path_concat (2, efidir_efi_boot, "bootriscv64.efi");
- make_image_fwdisk_abs (GRUB_INSTALL_PLATFORM_RISCV64_EFI, "riscv64-efi",
[PATCH 0/3] Add support for EFI file system transposition
Hello everyone, This series of patches adds file system transposition support, for UEFI boot media created with grub-mkrescue. File system transposition means the ability to take the content of a UEFI bootable media and copy it, at the file system level, to a partition that was independently created and formatted by the user, while preserving the ability of the media to boot in UEFI mode. We see this as a much needed improvement to GRUB when one of the core concept of EFI is to do away with the requirement to have to create boot media at the block level, one of the major pain points of BIOS systems' users. Currently, grub-mkrescue fails to meet the goal of UEFI file system transposition on 3 accounts: 1. It does not include file system support for FAT or NTFS, whereas these are the native file systems supported by UEFI (with FAT being mandatory per UEFI specs, and with NTFS being found more and more commonly on x64 commodity hardware such as, from my direct experience, about any motherboards that has been produced by ASUS, Gigabyte or Intel in the past 10 years). 2. It uses a efi.img to embed the UEFI bootloaders, but does not keep a copy of these bootloaders on the ISO9660 file system itself, with the end result that, when copying the media at the file system level, the '/efi/boot/' directory and its content is missing. 3. It relies on volume UUID to locate the boot media, a method that does not survive transposition when the content is copied to a newly user-created partition. The following patches fix each one of these issues. More specifically: 1. Adding fat and ntfs support can easily be added as additional modules and, considering that these are file systems natively supported on commonplace UEFI hardware, the benefits vastly outweighs the very limited increase in size. 2. Duplicating the 'efi.img' bootloaders onto the ISO9660 file system is also easily accomplished by dropping the use of a temporary directory to generate the 'efi.img' and instead moving copying that content to the ISO9660 root level. At this stage, we will point out that we consider it should really be the job of xorriso, rather than grub-mkrescue, to accomplish this duplication (hence why I am CC'ing Thomas), but we don't know the technical difficulties that result from trying to map back the content of a FAT image back onto the ISO9660 structure. There again, in terms of increase in size, we see the cost/benefit ratio as non issue. 3. Searching for the boot media is now be carried by looking for a '/.disk/.uuid' file rather than an actual partition UUID, as real world usage does show that relying on specific labels or UUIDs being assigned to specific partitions is actually a brittle solution. Note that we only carry out these alterations for EFI boot, and don't modify the existing BIOS/Legacy media search method. It should also be noted that the reason we chose a '.disk/' directory to place the UUID file is because '.disk/' has become a de-facto standard to place disk related content for Debian and Ubuntu, which, in the fragmented world of Linux distribution, is as good as a standard as you can get. We did briefly consider using '/System/', but decided against it as this latter directory is geared towards MacOS usage, and we see going with a more generic dot directory as a better approach. We have validated that, if the added content already contains a '.disk/' directory, then our new '.disk/#.uuid' file does get properly merged with that content. With these limited changes, grub-mkrescue can now be used to produce media that properly survives file system transposition, thereby satisfying one of the implicit goals of EFI of allowing end-users to carry out the creation of bootable EFI media at the file system level exclusively. Now, because this is relevant to this patchset (it's pretty much the motivation behind it), and because it is my understanding that some of the people creating ISOHybrid boot media, especially if they are using UNIX derivatives as their main work environments, are not always aware of the issues that arise from treating ISOHybrid as a glorified dd image, I will use this opportunity to elaborate on issues that arise from dealing with ISOHybrid media that lacks support for file system transposition. But first I should point out that these notes should not be construed as criticism of ISOHybrid per se, as we very much recognise the great benefits of ISOHybrid and the major technical achievements that went into it. We however do see it as important to try to debunk the idea that, once you have an ISOHybrid image that works well for both dd and optical, your job is done and remind people who place their trust in ISOHybrid image that, for the reasons highlighted below, having a working dd image is only half the job. And so, without further ado: * One massive real-world problem, that Linux-oriented people tend to ignore, is that for the mos
Re: [PATCH v3 1/2] json: Add function to unescape JSON-encoded strings
On Mon, 6 Jun 2022 07:18:28 +0200
Patrick Steinhardt wrote:
> On Sun, Jun 05, 2022 at 02:00:44PM -0500, Glenn Washburn wrote:
> > On Mon, 30 May 2022 18:01:01 +0200
> > 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 | 98 +++
> > > grub-core/lib/json/json.h | 12 +
> > > 2 files changed, 110 insertions(+)
> > >
> > > diff --git a/grub-core/lib/json/json.c b/grub-core/lib/json/json.c
> > > index 1c20c75ea..3e5e25f79 100644
> > > --- a/grub-core/lib/json/json.c
> > > +++ b/grub-core/lib/json/json.c
> > > @@ -262,3 +262,101 @@ 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, size_t *outlen, const char *in, size_t
> > > inlen)
> >
> > Why not grub_size_t instead of size_t?
> >
> > > +{
> > > + grub_err_t ret = GRUB_ERR_NONE;
> > > + size_t inpos, resultpos;
> >
> > Same here.
> >
> > > + char *result;
> > > +
> > > + result = grub_calloc (1, inlen + 1);
> >
> > Should check for grub_calloc failure.
> >
> > I like the idea of being able to do an inplace conversion if in ==
> > *out, which would save on memory and be safe as the out string length
> > should always be less than or equal to the length of the in string.
> > I'll admit its not really necessary for the current use-case.
> [snip]
>
> Yeah, I thought about this, and in fact my first iteration did exactly
> that. But it complicates the interface somewhat given that you now have
> to pass in the caller-allocated size, return the end result's size and
> also have to somehow handle NUL-terminating the result. So altogether it
> added complexity and felt a lot more fragile compared to just allocating
> the result array.
My thought was to have the caller pass in outlen pointing to the length
of the already allocated output array. No need for extra parameters.
Then outlen would be written to with the number of bytes written to the
output buffer. As for NULL terminating the result, no need to either,
just return an error indicating that the buffer was too small. I was
also thinking of keeping the current functionality, eg. output buffer
allocation, when *out == NULL. But yeah, this can be done some other
day when it would actually be useful.
Glenn
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 3/3] grub-core/kern/disk.c: handle LUKS2 devices
On Mon, 6 Jun 2022 07:32:40 +0200 Patrick Steinhardt wrote: > On Sun, Jun 05, 2022 at 01:43:18PM -0500, Glenn Washburn wrote: > > On Sun, 29 May 2022 09:09:38 +0200 > > Patrick Steinhardt wrote: > > > > > On Tue, May 10, 2022 at 10:55:52PM -0500, Glenn Washburn wrote: > > > > On Mon, 09 May 2022 22:27:30 +0200 > > > > Josselin Poiret wrote: > > > > > > > > > Hello everyone, > > > > > > > > > > Glenn Washburn writes: > > > > > > > > > > > I don't really like this, but it gets the job done and is a > > > > > > work-around > > > > > > for a peculiarity of the LUKS2 backend. The cheat mount code for > > > > > > cryptodisk does only calls scan() and not recover_key(). For LUKS1 > > > > > > scan > > > > > > will return a grub_cryptodisk_t with log_sector_size set, but LUKS2 > > > > > > will not. This is because for LUKS1 the log_sector_size is constant > > > > > > (LUKS1 also sets most of the other properties of the cryptodisk > > > > > > device, > > > > > > like crypto algorithm, because they are in the binary header). > > > > > > However, > > > > > > for LUKS2 the sector size (along with other properties) is in the > > > > > > json > > > > > > header, which isn't getting parsed in scan(). > > > > > > > > > > > > For single segment LUKS2 containers, scan() could get the sector > > > > > > size > > > > > > from the json segment object. The LUKS2 spec says that normal LUKS2 > > > > > > devices are single segment[1], so this should work in the the cases > > > > > > the > > > > > > care about (currently). scan() would not be able to fill in the > > > > > > other > > > > > > properties, like crypto algorithm, because that depends on the > > > > > > keyslot > > > > > > used, which needs key recovery to be determined. To avoid parsing > > > > > > the > > > > > > json data twice, once in scan() and once in recover_key(), which > > > > > > should > > > > > > be avoided, the parsed json object could be put in the > > > > > > grub_cryptodisk_t > > > > > > in scan(), and used and freed in recover_key(). We'd probably also > > > > > > want > > > > > > to add a way for grub_cryptodisk_t objects to get cleaned up by the > > > > > > backend using them, so that the json object could be freed even if > > > > > > recover_key() is never called. > > > > > > > > > > > > I think the above is the real fix, a moderate amount more work, and > > > > > > not > > > > > > something I'd expect Pierre-Louis to take up. So if we're not going > > > > > > to > > > > > > do this to get this functionality to work, we'll need a hack to get > > > > > > it > > > > > > working. However, I'd prefer a different one. > > > > > > > > > > > > I've not tested this, but it seems to me that we can set the > > > > > > log_sector_size field to GRUB_DISK_SECTOR_BITS _if_ it equals zero > > > > > > in > > > > > > grub_cryptodisk_cheat_insert(). This limits the hack to only GRUB > > > > > > host/user-space code. > > > > > > > > > > Regarding these last lines, it's also possible to directly ask dm for > > > > > the actual sector size when cheatmounting, as well as the crypto > > > > > algorithm, bypassing the whole issue of parsing the json and finding > > > > > the > > > > > right slot. This is roughly what's done in patch 2 of [1], maybe this > > > > > workaround would be more to your liking? > > > > > > > > Hah! Yes, thanks for reminding me. I forgot I'd suggested this and its > > > > a better approach than what I suggested above. And probably the one I'd > > > > support, but I need to more thoroughly take a look at it. > > > > > > > > > I've distributed this patch to several people that were having issues > > > > > on > > > > > GNU Guix and they've been happily using LUKS2 with GRUB with it. > > > > > > > > Yes, this does work and too much of a hack as-is. Regardless, your > > > > contribution is appreciated. I'd like to get your patch with the GRUB fs > > > > tests merged. Do you want to make the changes I suggested and send a new > > > > patch here? If not, at some point I'll probably make them myself and > > > > submit it to the list. > > > > > > > > > [1] > > > > > https://lists.gnu.org/archive/html/grub-devel/2021-12/msg00079.html > > > > > (20211211122945.6326-1-...@jpoiret.xyz) > > > > > > > > Glenn > > > > > > I very much agree that we should land the test-patches regardless of > > > what happens to the rest: they cover an important test gap. > > > > > > Other than that the patches look sane to me. The biggest question to me > > > is which of the three patch series we want to include in the end: > > > > > > - Yours has the extra benefit of added tests, but these can go in > > > independently. > > > > > > - Josselin's patches [1] have the benefit that they try to derive a > > > "proper" sector size via device-mapper. > > > > > > - My own patches [2] include two additional patches: one to strip > > > dashes of the UUID so that findfs is easier to use and the same > > > across LU
Re: [PATCH v4 1/2] json: Add function to unescape JSON-encoded strings
On Mon, 6 Jun 2022 07:28:56 +0200
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
Reviewed-by: Glenn Washburn
Glenn
> ---
> grub-core/lib/json/json.c | 101 ++
> grub-core/lib/json/json.h | 12 +
> 2 files changed, 113 insertions(+)
>
> diff --git a/grub-core/lib/json/json.c b/grub-core/lib/json/json.c
> index 1c20c75ea..adb4747a4 100644
> --- a/grub-core/lib/json/json.c
> +++ b/grub-core/lib/json/json.c
> @@ -262,3 +262,104 @@ 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 || !outlen)
> +return grub_error (GRUB_ERR_BAD_ARGUMENT, "Output parameters are not
> set");
> +
> + result = grub_calloc (1, inlen + 1);
> + if (!result)
> +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, "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':
> + {
> + unsigned char values[4] = {0};
> + int i;
> +
> + inpos++;
> + if (inpos + ARRAY_SIZE(values) > inlen)
> + {
> + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unicode
> sequence too short");
> + goto err;
> + }
> +
> + for (i = 0; i < 4; 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,
> + "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, "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#se
Re: [PATCH v4 2/2] luks2: Fix decoding of digests and salts with escaped chars
On Mon, 6 Jun 2022 07:29:00 +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
Reviewed-by: Glenn Washburn
Glenn
> ---
> 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);
> + 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;
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Add some randomness to TCP source port selection.
256 is a bad modulo. A prime would be a much better one for those purposes. Also get_time_ms counts up from arbitrary point in time, often boot. I suggest using some combination of etc and get_time to seed an LFSR algorithm Le lun. 6 juin 2022, 18:37, Robert LeBlanc a écrit : > GRUB uses a static source TCP port and increments for each new > connection. When rapidly restarting GRUB this can cause issues with some > firewalls that suspect that a reply attack is happening. In addition > GRUB does not ACK the last FIN,ACK when booting the kernel and initrd > from HTTP for example. This cause the remote HTTP server to keep the TCP > session in TIME_WAIT and reject new connections from the same port > combination when restarted quickly. This helps to work around both > problems by shifting the source port by a small amount based on time. > > The missing final ACK should also be addressed, but I'm not sure how to > resolve that. > > Signed-off-by: Robert LeBlanc > --- > grub-core/net/tcp.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/grub-core/net/tcp.c b/grub-core/net/tcp.c > index 93dee0caa..2eefd3168 100644 > --- a/grub-core/net/tcp.c > +++ b/grub-core/net/tcp.c > @@ -569,7 +569,7 @@ grub_net_tcp_open (char *server, >struct grub_net_network_level_interface *inf; >grub_net_network_level_address_t gateway; >grub_net_tcp_socket_t socket; > - static grub_uint16_t in_port = 21550; > + grub_uint16_t in_port = 21550 + grub_get_time_ms () % 256; >struct grub_net_buff *nb; >struct tcphdr *tcph; >int i; > @@ -603,7 +603,7 @@ grub_net_tcp_open (char *server, >socket->inf = inf; >socket->out_nla = addr; >socket->ll_target_addr = ll_target_addr; > - socket->in_port = in_port++; > + socket->in_port = in_port; >socket->recv_hook = recv_hook; >socket->error_hook = error_hook; >socket->fin_hook = fin_hook; > -- > 2.35.1 > > > ___ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel > ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Add some randomness to TCP source port selection.
Le lun. 6 juin 2022, 19:25, Vladimir 'phcoder' Serbinenko a écrit : > 256 is a bad modulo. A prime would be a much better one for those > purposes. Also get_time_ms counts up from arbitrary point in time, often > boot. I suggest using some combination of etc > RTC, not etc > > and get_time to seed an LFSR algorithm > > Le lun. 6 juin 2022, 18:37, Robert LeBlanc a > écrit : > >> GRUB uses a static source TCP port and increments for each new >> connection. When rapidly restarting GRUB this can cause issues with some >> firewalls that suspect that a reply attack is happening. In addition >> GRUB does not ACK the last FIN,ACK when booting the kernel and initrd >> from HTTP for example. This cause the remote HTTP server to keep the TCP >> session in TIME_WAIT and reject new connections from the same port >> combination when restarted quickly. This helps to work around both >> problems by shifting the source port by a small amount based on time. >> >> The missing final ACK should also be addressed, but I'm not sure how to >> resolve that. >> >> Signed-off-by: Robert LeBlanc >> --- >> grub-core/net/tcp.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/grub-core/net/tcp.c b/grub-core/net/tcp.c >> index 93dee0caa..2eefd3168 100644 >> --- a/grub-core/net/tcp.c >> +++ b/grub-core/net/tcp.c >> @@ -569,7 +569,7 @@ grub_net_tcp_open (char *server, >>struct grub_net_network_level_interface *inf; >>grub_net_network_level_address_t gateway; >>grub_net_tcp_socket_t socket; >> - static grub_uint16_t in_port = 21550; >> + grub_uint16_t in_port = 21550 + grub_get_time_ms () % 256; >>struct grub_net_buff *nb; >>struct tcphdr *tcph; >>int i; >> @@ -603,7 +603,7 @@ grub_net_tcp_open (char *server, >>socket->inf = inf; >>socket->out_nla = addr; >>socket->ll_target_addr = ll_target_addr; >> - socket->in_port = in_port++; >> + socket->in_port = in_port; >>socket->recv_hook = recv_hook; >>socket->error_hook = error_hook; >>socket->fin_hook = fin_hook; >> -- >> 2.35.1 >> >> >> ___ >> Grub-devel mailing list >> Grub-devel@gnu.org >> https://lists.gnu.org/mailman/listinfo/grub-devel >> > ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: GRUB coverity fixes for CIDs 314020 and 314023
On Fri, Jun 03, 2022 at 02:12:00PM +0100, Darren Kenny wrote: > On Thursday, 2022-06-02 at 15:18:25 UTC, Jagannathan Raman wrote: > > Hi, > > > > This series addresses a couple of untrusted loop bounds flagged by > > Coverity in "grub-core/fs/zfs". Both the bugs addressed in this series > > are of the same type - caused by downcast of pointer from a strict type > > to a less strict type. > > > > Please share your thoughts on this. > > > > These changes look good to me, thanks for looking at them Jag. > > Reviewed-by: Darren Kenny Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v4] efidisk: pass buffers with higher alignment
On Tue, May 31, 2022 at 06:10:42PM +0200, Heinrich Schuchardt wrote: > On 5/31/22 17:53, Stefan Agner wrote: > > Some devices report IoAlign values but seem to require buffers with > > higher alignment. > > > > The UEFI specification is saying: "IoAlign values of 0 and 1 mean that > > the buffer can be placed anywhere in memory. Otherwise, IoAlign must > > be a power of 2, and the requirement is that the start address of a > > buffer must be evenly divisible by IoAlign with no remainder." > > > > Some devices report IoAlign of 2, however seem to require 4 bytes > > aligned buffers. It seems that this got misinterpreted by some vendors > > assuming IoAlign is 2^IoAlign. There is also such a hint in an example > > in earlier versions of the Driver Writer's Guide: > > ScsiPassThruMode.IoAlign = 2; // Data must be alligned on 4-byte boundary > > > > Some devices report no alignment requirements at all but seem to read > > corrupted data or report read errors when passing unaligned buffers. > > > > Work around by using an alignment of at least BlockSize (typically 512 > > bytes) in any case. If IoAlign (interpreted as per UEFI specification) > > requests a higher alignment than BlockSize, follow IoAlign still. > > > > Note: The problem has only noticed with compressed squashfs. It seems > > that ext4 (and presumably other file system drivers) pass buffers with > > a higher alignment already. > > > > Signed-off-by: Stefan Agner > > Acked-by: Heinrich Schuchardt Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
