From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1kmlkt-00052u-6e for mharc-grub-devel@gnu.org; Tue, 08 Dec 2020 17:46:47 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:36882) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kmlkj-0004rJ-Pl for grub-devel@gnu.org; Tue, 08 Dec 2020 17:46:37 -0500 Received: from mail-oi1-x242.google.com ([2607:f8b0:4864:20::242]:35215) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kmlkZ-0000lQ-MV for grub-devel@gnu.org; Tue, 08 Dec 2020 17:46:37 -0500 Received: by mail-oi1-x242.google.com with SMTP id s2so349265oij.2 for ; Tue, 08 Dec 2020 14:46:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=efficientek-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=u/BTEKh2I4cpaZ+PssemALKWR+uRz7XZgYbY+zOGUcA=; b=uaBGJF7HV4SMgDNb2zjmDp5T4Vt17tR+FriIdv0m+ticlfFZm7du/jLapPktkEHjkZ G6WF7kerrYhNLQtcnorZDsBBPvea718UYe0zsvONilenM+gVQmM9qPrkg92ujSukWsVz bLOwdQOJbu4KEJjsw4VYYPszqdcrBUMWGKoQ25iNG+L94EFVzukL28G64jmXnuQHiT6i wzIybq9iR6mJwKGUwFPN4G7u1Qtj2VcTkTpQX6e2XAKYjd8/TpuusHfNwzMzkd3Uj+HJ BT23EAha58823jBaE/LkbTkmvTasOfRk6XqqXGPAwtcnTvEsIeUtiv3r9cU1Qz8v940/ AhVg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=u/BTEKh2I4cpaZ+PssemALKWR+uRz7XZgYbY+zOGUcA=; b=lxB/14QNpa9Mfh6j7WoTHgFw6e9JbQpJkRDxle5yO0jddW2U9pSj5PAd7N0uYh7OyN QiCSfri6c2eT6/SfSpCe4IHppiFqSDCY+ajZjIyUlxbxHNLIRC5InsE8jonG1PaNZhAH M261+FW7FssL99oKO3kxgY1yP3EU2XuPQTVE00e6yGbcpAr6duaylXx4wTe3hy01coj0 nS+M27q34M9bSX0KHumGAEZWb50EwDzBJT8ZmK6U70okuBZlfsURGgBKEYOBFxov/1dQ Vfkw2EXt73fXxE9TgWSTjQEMEvd1ZQxFTNRkHKgYUgHBlnvKFHheTTp8Ox30cl7/l2Y5 3Qdw== X-Gm-Message-State: AOAM532K/cP0HYIvU+XmCW4Klp3vFz0hM7CWQsWv1Ae5H/mbltLnKFQM IMOO7Tv4hYIb6qPG6x0H7DSWkiPS8ToEkg== X-Google-Smtp-Source: ABdhPJwj5u8QArIUgOn/0jb57+MOlq5kqjutaue7gYi3ODZ1qOUKh2C2uJz6wMMHyyUXEHv1d/ZDLA== X-Received: by 2002:aca:53ca:: with SMTP id h193mr117685oib.122.1607467585965; Tue, 08 Dec 2020 14:46:25 -0800 (PST) Received: from crass-HP-ZBook-15-G2.attlocal.net ([136.49.211.192]) by smtp.gmail.com with ESMTPSA id v8sm45538otp.10.2020.12.08.14.46.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 08 Dec 2020 14:46:25 -0800 (PST) From: Glenn Washburn To: grub-devel@gnu.org Cc: Patrick Steinhardt , Daniel Kiper , Glenn Washburn Subject: [PATCH v8 14/18] luks2: Better error handling when setting up the cryptodisk Date: Tue, 8 Dec 2020 16:45:45 -0600 Message-Id: <9fbd815f9360ed2faa45c386d4ed103103284417.1607466704.git.development@efficientek.com> X-Mailer: git-send-email 2.27.0 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Received-SPF: pass client-ip=2607:f8b0:4864:20::242; envelope-from=development@efficientek.com; helo=mail-oi1-x242.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 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 08 Dec 2020 22:46:38 -0000 First, check to make sure that source disk has a known size. If not, print debug 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. Since I doubt booting from a LUKS2 volume on a cd is a big use case, we'll error until someone complains. Do some sanity checking on data coming from the luks 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, we want to continue to try to use this key, but only allow access up to the end of the source disk. Signed-off-by: Glenn Washburn --- grub-core/disk/luks2.c | 84 ++++++++++++++++++++++++++++++++++++++++-- include/grub/disk.h | 17 +++++++++ 2 files changed, 98 insertions(+), 3 deletions(-) diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c index 9abcb1c94..8cb11e899 100644 --- a/grub-core/disk/luks2.c +++ b/grub-core/disk/luks2.c @@ -600,12 +600,26 @@ luks2_recover_key (grub_disk_t source, goto err; } + if (source->total_sectors == GRUB_DISK_SIZE_UNKNOWN) + { + /* FIXME: Allow use of source disk, and maybe cause errors in read. */ + grub_dprintf ("luks2", "Source disk %s has an unknown size, " + "conservatively returning error\n", source->name); + ret = grub_error (GRUB_ERR_BUG, "Unknown size of luks2 source device"); + goto err; + } + /* Try all keyslot */ for (json_idx = 0; json_idx < size; json_idx++) { + typeof(source->total_sectors) max_crypt_sectors = 0; + + grub_errno = GRUB_ERR_NONE; ret = luks2_get_keyslot (&keyslot, &digest, &segment, json, json_idx); if (ret) goto err; + if (grub_errno != GRUB_ERR_NONE) + grub_dprintf ("luks2", "Ignoring unhandled error %d from luks2_get_keyslot\n", grub_errno); if (keyslot.priority == 0) { @@ -619,11 +633,75 @@ luks2_recover_key (grub_disk_t source, crypt->offset_sectors = grub_divmod64 (segment.offset, segment.sector_size, NULL); crypt->log_sector_size = sizeof (unsigned int) * 8 - __builtin_clz ((unsigned int) segment.sector_size) - 1; + /* Set to the source disk size, which is the maximum we allow. */ + max_crypt_sectors = grub_disk_convert_sector(source, + source->total_sectors, + crypt->log_sector_size); + + if (max_crypt_sectors < crypt->offset_sectors) + { + grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" has offset" + " %"PRIuGRUB_UINT64_T" which is greater than" + " source disk size %"PRIuGRUB_UINT64_T"," + " skipping\n", + segment.idx, crypt->offset_sectors, + max_crypt_sectors); + continue; + } + if (grub_strcmp (segment.size, "dynamic") == 0) - crypt->total_sectors = (grub_disk_get_size (source) >> (crypt->log_sector_size - source->log_sector_size)) - - crypt->offset_sectors; + crypt->total_sectors = max_crypt_sectors - crypt->offset_sectors; else - crypt->total_sectors = grub_strtoull (segment.size, NULL, 10) >> crypt->log_sector_size; + { + grub_errno = GRUB_ERR_NONE; + /* Convert segment.size to sectors, rounding up to nearest sector */ + crypt->total_sectors = grub_strtoull (segment.size, NULL, 10); + crypt->total_sectors = ALIGN_UP (crypt->total_sectors, + 1 << crypt->log_sector_size); + crypt->total_sectors >>= crypt->log_sector_size; + + if (grub_errno == GRUB_ERR_NONE) + ; + else if (grub_errno == GRUB_ERR_BAD_NUMBER) + { + grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" size" + " \"%s\" is not a parsable number\n", + segment.idx, segment.size); + continue; + } + else if (grub_errno == GRUB_ERR_OUT_OF_RANGE) + { + /* + * There was an overflow in parsing segment.size, so disk must + * be very large or the string is incorrect. + */ + grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" size" + " %s overflowed 64-bit unsigned integer," + " the end of the crypto device will be" + " inaccessible\n", + segment.idx, segment.size); + if (crypt->total_sectors > max_crypt_sectors) + crypt->total_sectors = max_crypt_sectors; + } + } + + if (crypt->total_sectors == 0) + { + grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" has zero" + " sectors, skipping\n", + segment.idx); + continue; + } + else if (max_crypt_sectors < (crypt->offset_sectors + crypt->total_sectors)) + { + grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" has last" + " data position greater than source disk size," + " the end of the crypto device will be" + " inaccessible\n", + segment.idx); + /* Allow decryption up to the end of the source disk. */ + crypt->total_sectors = max_crypt_sectors - crypt->offset_sectors; + } ret = luks2_decrypt_key (candidate_key, source, crypt, &keyslot, (const grub_uint8_t *) passphrase, grub_strlen (passphrase)); diff --git a/include/grub/disk.h b/include/grub/disk.h index 0fb727d3d..f9227f285 100644 --- a/include/grub/disk.h +++ b/include/grub/disk.h @@ -27,6 +27,8 @@ #include /* For NULL. */ #include +/* For ALIGN_UP. */ +#include /* These are used to set a device id. When you add a new disk device, you must define a new id for it here. */ @@ -174,6 +176,21 @@ typedef struct grub_disk_memberlist *grub_disk_memberlist_t; /* Return value of grub_disk_native_sectors() in case disk size is unknown. */ #define GRUB_DISK_SIZE_UNKNOWN 0xffffffffffffffffULL +/* Convert sector number from disk sized sectors to a log-size sized sector. */ +static inline grub_disk_addr_t +grub_disk_convert_sector (grub_disk_t disk, + grub_disk_addr_t sector, + grub_size_t log_sector_size) +{ + if (disk->log_sector_size < log_sector_size) + { + sector = ALIGN_UP (sector, 1 << (log_sector_size / disk->log_sector_size)); + return sector >> (log_sector_size - disk->log_sector_size); + } + else + return sector << (disk->log_sector_size - log_sector_size); +} + /* Convert to GRUB native disk sized sector from disk sized sector. */ static inline grub_disk_addr_t grub_disk_from_native_sector (grub_disk_t disk, grub_disk_addr_t sector) -- 2.27.0