From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1nqibs-0001dO-8Q for mharc-grub-devel@gnu.org; Mon, 16 May 2022 17:50:36 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:41248) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nqibc-0001ac-2n for grub-devel@gnu.org; Mon, 16 May 2022 17:50:23 -0400 Received: from mail-vs1-xe29.google.com ([2607:f8b0:4864:20::e29]:46687) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1nqibX-00086J-SA for grub-devel@gnu.org; Mon, 16 May 2022 17:50:19 -0400 Received: by mail-vs1-xe29.google.com with SMTP id z144so16911488vsz.13 for ; Mon, 16 May 2022 14:50:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=efficientek-com.20210112.gappssmtp.com; s=20210112; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=2noQ3k3yH/bwynNq3QTqYO5wO1uvxa7j+tkrgmiQKsA=; b=ZB3eo/8PBLJS3P07DnN5jVf4XJwKFuHkENxu/JVVrbbPocE/lgHDcyjXV04rmUaAfM fJjjlk5+oJ178GBIcVyTWXwrMfyjQ05awh0MJLqsoOISDQl3hk1/PaKFBdbBWUqx09Ge sRiIHCqTPttRpbUg9qFxLltREFln0wsrFJYLoqqndXj5/YN5iYeaE/7e0BV3R2CZnWWy X2opPF6jlQeZZv2c+BkC0Ckv3ZbEXtJ7/h0esm1gA9g1QhMmrOm5Yb5kZ1UZlzQ6w8f9 KvCrWN/XTdvL2uchf6Y/28HUDS3qaE/wQzpapXbYaSkWEYXf+VvaoDZJVDGT1U2wvAsC NIxQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=2noQ3k3yH/bwynNq3QTqYO5wO1uvxa7j+tkrgmiQKsA=; b=HkMbHBhKK5nM59PK5v3PXy8egVxBSebKPQhtYOsTSCvijQs4039P/7jqBiuteakehk HMlfh15KqtIVoBYCl8yzC/2W/FCiFKzkTmkpm75gLtlLnTXwYu8kLKeZp/rzohg8nKTo 872z3hJzZnLb4MnYdjWTYK//bWqf+L8Ad1RC+XTYta3uKR+FXpuc1znHpqtQYrbIwa5P tPIU6hW6ISYF5LiMzbf6FTomaKxbER14UTx9nhQy+tvjourjCRQzKG14Q+4unQpUf7ZN Oi2QhENF7OqMPKgfvv2r5e+pt17YSvUhpJ7gME0cwfQOMZjii9+ESzkv/u4nm7VlVZ+T ZZHg== X-Gm-Message-State: AOAM532KFMJgOkY9geRACKOqYXaQcHjQrch8W/lJgC28gVQ1O96haAiV uyvoXufOXuMzHifVPr6elMXBh9Mzf2DyKbKl X-Google-Smtp-Source: ABdhPJz/Q5xAd7HglAV19ZPNRIRE7/BcPleTSEo1wXXqrx0A62315B+at9aG/Ll9HModRUzdwmNqZQ== X-Received: by 2002:a67:f94d:0:b0:32d:2ac7:4bb4 with SMTP id u13-20020a67f94d000000b0032d2ac74bb4mr6870375vsq.23.1652737812221; Mon, 16 May 2022 14:50:12 -0700 (PDT) Received: from localhost.localdomain ([37.218.244.249]) by smtp.gmail.com with ESMTPSA id z7-20020a673307000000b0032d275e691esm1018581vsz.30.2022.05.16.14.50.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 16 May 2022 14:50:11 -0700 (PDT) From: Glenn Washburn To: grub-devel@gnu.org, Daniel Kiper Cc: Denis 'GNUtoo' Carikli , Patrick Steinhardt , John Lane , Glenn Washburn Subject: [PATCH v2 0/3] Cryptomount detached headers Date: Mon, 16 May 2022 16:49:45 -0500 Message-Id: X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Received-SPF: pass client-ip=2607:f8b0:4864:20::e29; envelope-from=development@efficientek.com; helo=mail-vs1-xe29.google.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 May 2022 21:50:24 -0000 Note: Description here has changed since last time, the last few sentences, which describe a method for allowing backends to bypass the read hook on the source disk. Updates since v1: * Add marge comment block describing at a high level the read hook operation and assumptions. * Add code to unset read hook on source disk when exiting grub_cryptodisk_scan_device_real(). A couple return calls were converted into gotos so that logical flows to one exit point. This patch series is, I believe, a better approach to supporting detached headers for cryptomount and backends. This series will probably not apply cleanly without the changes from the recent series entitled "[PATCH 0/4] Cryptomount keyfile support". But its only because they touch code in the same vicinity, not because there's any real dependency. Conceptually the approach is different than the previous detach header series because this one uses the disk objects read hook to hook reads to the disk in scan() and recover_key() such that if there is an associated header file, the hook will cause the read to return data from the header file instead of from the source disk. For this the read hook implementation needed to be upgaded because prior it didn't get the read buffer sent from the read caller and so could not modify its contents. Patch #1 updates the hook accordingly and all instances of its use, but doesn't functionally change how GRUB operates. The second patch adds the header option to cryptomount and the read hook to the source disk during the scan() and recover_key() stages. The benefit of this approach is its simpler/less code and does not require the modification of backends, except to potentially cause a failure in scan() if the backend doesn't support the current implementation of detached headers, as with the GELI backend. This implementation requires that the crypto volume header reside at the beginning of the volume. GELI has its header at the end, which is why it can not currently be supported. In theory, GELI could be supported if extra assumptions about its source access pattern during scan() and recovery_key() were made. I don't use GELI, no one seems to be asking for GELI detached headers, and I don't think that GELI even support detached headers with the official tools. So for me, not supporting crypto volumes with headers at the end of the disk is not a big deal. With the patch series each backend gets the header file through cargs, so it can implement detached headers by solely operating on that file instead of the hooked source disk. In the the future, a flag can be added to the cryptodisk_dev_t that backends can sent when registering to enabled/disable the use of the read hook, if the backend needs to read from both the detached header file and the source disk. Glenn Glenn Washburn (3): disk: Allow read hook callback to take read buffer to potentially modify it cryptodisk: Add support for using detached header files docs: Add documentation on detached header option to cryptomount docs/grub.texi | 13 +++-- grub-core/commands/blocklist.c | 10 ++-- grub-core/commands/loadenv.c | 8 +-- grub-core/commands/testload.c | 4 +- grub-core/disk/cryptodisk.c | 93 ++++++++++++++++++++++++++++++++-- grub-core/disk/geli.c | 4 ++ grub-core/fs/hfspluscomp.c | 4 +- grub-core/fs/ntfscomp.c | 14 ++--- grub-core/kern/disk.c | 12 ++--- grub-core/lib/progress.c | 11 ++-- grub-core/net/net.c | 2 +- include/grub/cryptodisk.h | 2 + include/grub/disk.h | 6 +-- include/grub/file.h | 2 + 14 files changed, 147 insertions(+), 38 deletions(-) Range-diff against v1: 1: b3672a4cf = 1: ad4a48225 disk: Allow read hook callback to take read buffer to potentially modify it 2: a3ffa4738 ! 2: 25db3635f cryptodisk: Add --header option to cryptomount to support detached headers @@ Metadata Author: Glenn Washburn ## Commit message ## - cryptodisk: Add --header option to cryptomount to support detached headers + cryptodisk: Add support for using detached header files - Add a --header (short -H) option to cryptomount which takes a file argument. - Using the improved read hook, setup a read hook on the source device which - will read from the given header file during the scan and recovery cryptodisk - backend functions. This makes supporting detached headers transparent to the - backend, so long as the attached header is located at the start of the - source disk. This is not the case for GELI volumes, which have the header at - the end of the volume. So GELI will return an error if a detached header is - specified. + Using the disk read hook mechanism, setup a read hook on the source disk + which will read from the given header file during the scan and recovery + cryptodisk backend functions. Disk read hooks are executed after the data + has been read from the disk. This is okay, because the read hook is given + the read buffer before its sent back to the caller. In this case, the hook + can then overwrite the data read from the disk device with data from the + header file sent in as the read hook data. This is transparent to the + read caller. Since the callers of this function have just opened the + source disk, there are no current read hooks, so there's no need to + save/restore them nor consider if they should be called or not. + + This hook assumes that the header is at the start of the volume, which + is not the case for some formats (eg. GELI). So GELI will return an + error if a detached header is specified. It also can only be used + with formats where the detached header file can be written to the + first blocks of the volume and the volume could still be unlocked. + So the header file can not be formatted differently from the on-disk + header. If these assumpts are not met, detached header file processing + must be specially handled in the cryptodisk backend module. + + The hook will be called potentially many times by a backend. This is fine + because of the assumptions mentioned and the read hook reads from absolute + offsets and is stateless. + + Also add a --header (short -H) option to cryptomount which takes a file + argument. ## grub-core/disk/cryptodisk.c ## +@@ grub-core/disk/cryptodisk.c: enum + OPTION_PASSWORD, + OPTION_KEYFILE, + OPTION_KEYFILE_OFFSET, +- OPTION_KEYFILE_SIZE ++ OPTION_KEYFILE_SIZE, ++ OPTION_HEADER + }; + + static const struct grub_arg_option options[] = @@ grub-core/disk/cryptodisk.c: static const struct grub_arg_option options[] = {"key-file", 'k', 0, N_("Key file"), 0, ARG_TYPE_STRING}, {"keyfile-offset", 'O', 0, N_("Key file offset (bytes)"), 0, ARG_TYPE_INT}, @@ grub-core/disk/cryptodisk.c: static const struct grub_arg_option options[] = +struct cryptodisk_read_hook_ctx +{ -+ grub_disk_read_hook_t prev_read_hook; -+ void *prev_read_hook_data; + grub_file_t hdr_file; +}; +typedef struct cryptodisk_read_hook_ctx *cryptodisk_read_hook_ctx_t; @@ grub-core/disk/cryptodisk.c: cryptodisk_close (grub_cryptodisk_t dev) + cryptodisk_read_hook_ctx_t ctx = data; + + if (ctx->hdr_file == NULL) -+ return GRUB_ERR_NONE; ++ return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("header file not found")); + + if (grub_file_seek (ctx->hdr_file, + (sector * GRUB_DISK_SECTOR_SIZE) + offset) @@ grub-core/disk/cryptodisk.c: cryptodisk_close (grub_cryptodisk_t dev) + return grub_errno; + + if (grub_file_read (ctx->hdr_file, buf, length) != (grub_ssize_t) length) -+ return grub_errno; -+ -+ if (ctx->prev_read_hook != NULL) -+ ret = ctx->prev_read_hook(sector, offset, length, buf, -+ ctx->prev_read_hook_data); ++ { ++ if (grub_errno == GRUB_ERR_NONE) ++ grub_error (GRUB_ERR_OUT_OF_RANGE, N_("header file too small")); ++ return grub_errno; ++ } + + return ret; +} @@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device_real (const char *name, + if (cargs->hdr_file != NULL) + { ++ /* ++ * Set read hook to read header from a file instead of the source disk. ++ * Disk read hooks are executed after the data has been read from the ++ * disk. This is okay, because the read hook is given the read buffer ++ * before its sent back to the caller. In this case, the hook can then ++ * overwrite the data read from the disk device with data from the ++ * header file sent in as the read hook data. This is transparent to the ++ * read caller. Since the callers of this function have just opened the ++ * source disk, there are no current read hooks, so there's no need to ++ * save/restore them nor consider if they should be called or not. ++ * ++ * This hook assumes that the header is at the start of the volume, which ++ * is not the case for some formats (eg. GELI). It also can only be used ++ * with formats where the detached header file can be written to the ++ * first blocks of the volume and the volume could still be unlocked. ++ * So the header file can not be formatted differently from the on-disk ++ * header. If these assumpts are not met, detached header file processing ++ * must be specially handled in the cryptodisk backend module. ++ * ++ * This hook needs only be set once and will be called potentially many ++ * times by a backend. This is fine because of the assumptions mentioned ++ * and the read hook reads from absolute offsets and is stateless. ++ */ + read_hook_data.hdr_file = cargs->hdr_file; -+ -+ if (source->read_hook != NULL) -+ { -+ read_hook_data.prev_read_hook = source->read_hook; -+ read_hook_data.prev_read_hook_data = source->read_hook_data; -+ } -+ + source->read_hook = cryptodisk_read_hook; + source->read_hook_data = (void *) &read_hook_data; + } + FOR_CRYPTODISK_DEVS (cr) { ++ /* ++ * Loop through each cryptodisk backend that is registered (ie. loaded). ++ * If the scan returns NULL, then the backend being tested does not ++ * recognize the source disk, so move on to the next backend. ++ */ dev = cr->scan (source, cargs); + if (grub_errno) +- return NULL; ++ goto error_no_close; + if (!dev) + continue; + +@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device_real (const char *name, + + cargs->key_data = grub_malloc (GRUB_CRYPTODISK_MAX_PASSPHRASE); + if (cargs->key_data == NULL) +- return NULL; ++ goto error_no_close; + + if (!grub_password_get ((char *) cargs->key_data, GRUB_CRYPTODISK_MAX_PASSPHRASE)) + { @@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device_real (const char *name, + + error: + cryptodisk_close (dev); ++ error_no_close: dev = NULL; cleanup: -+ if (read_hook_data.prev_read_hook != NULL) -+ { -+ source->read_hook = read_hook_data.prev_read_hook; -+ source->read_hook_data = read_hook_data.prev_read_hook_data; -+ } ++ if (cargs->hdr_file != NULL) ++ source->read_hook = NULL; ++ if (askpass) { cargs->key_len = 0; @@ grub-core/disk/cryptodisk.c: grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args) - return grub_error (GRUB_ERR_FILE_READ_ERROR, (N_("reading key file"))); + return grub_error (GRUB_ERR_FILE_READ_ERROR, (N_("failed to read key file"))); } -+ if (state[7].set) /* header */ ++ if (state[OPTION_HEADER].set) /* header */ + { -+ if (state[0].set) ++ if (state[OPTION_UUID].set) + return grub_error (GRUB_ERR_BAD_ARGUMENT, + N_("cannot use UUID lookup with detached header")); + -+ cargs.hdr_file = grub_file_open (state[7].arg, ++ cargs.hdr_file = grub_file_open (state[OPTION_HEADER].arg, + GRUB_FILE_TYPE_CRYPTODISK_DETACHED_HEADER); + if (cargs.hdr_file == NULL) + return grub_errno; + } + - if (state[0].set) /* uuid */ + if (state[OPTION_UUID].set) /* uuid */ { int found_uuid; @@ grub-core/disk/cryptodisk.c: GRUB_MOD_INIT (cryptodisk) 3: 01c30f477 = 3: a0bd5337e docs: Add documentation on detached header option to cryptomount -- 2.34.1