From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1kh4Ku-0008L9-6e for mharc-grub-devel@gnu.org; Mon, 23 Nov 2020 00:24:24 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:40626) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kh4Kn-0008Go-6s for grub-devel@gnu.org; Mon, 23 Nov 2020 00:24:17 -0500 Received: from mail-wr1-x443.google.com ([2a00:1450:4864:20::443]:34023) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kh4Kl-0006XE-8v for grub-devel@gnu.org; Mon, 23 Nov 2020 00:24:16 -0500 Received: by mail-wr1-x443.google.com with SMTP id r17so17380814wrw.1 for ; Sun, 22 Nov 2020 21:24:14 -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=TkVA28xFvS6p/S4wnJhiUVBcFiCAdTQiviD79UwpcxU=; b=MlN4KIlAI532Zwth8++hYh70s9cgaEAyLbbVthiZWXgE27zPdrJFnbsF5gxAxoT9E1 0BjRIOs0MMVhCqXrwwKtyYdZydf6t90aHQ7sayIfa9QfPMdnTMkjgtk8ZaJdyL0CKtU1 /DuIKbByOqmIH356s9yj/xhjDyqnH7Q8TqLzu89Zlq/u/PUhxzvn6toE6+LOdp/Pw5gt hoMteQqmfGBMUb6fxj/AyJct8wtqDqjjENWOEQGFo3RvUvBR7mZ6ySHMwJJObVOqqsME cgT8loI0H0ZynenkdqFmaOirPJGsrnPh+JJn7X1aCir13z1n4i4P3BgzF6EzgQxnptCS f2+g== 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=TkVA28xFvS6p/S4wnJhiUVBcFiCAdTQiviD79UwpcxU=; b=JTQm6q37U+PoHnA5agv6cjlwEqgn1/MgMJRKR54efiJs/1SJlVxCiNOJUIJ68T2JzQ E5w1gvhlANvroqL8PhYEFtz9CGGOo+l0A+mP+MGVzMZ3q59r6xM2ZUe4cRJG2U4VK1iZ JC36zfmIrrzwCMyME/5NP9siLqUSGYIZGTEXij0oMQLBP01P/ZAzZaUHhpLoLY4HeAEw cLi9VH+5R/gEhG2wzWiLpNcpL1CYr2NryzD6kskIiijpO9Tq+7ScTBg3GVCJ0zeF/okc DI7sV+YnXjdfdkMDn2puDHHeFLKgvS4gMbjQIGo2yYmD/xlm3xadsHe56ICbgBlNlMYy AaZQ== X-Gm-Message-State: AOAM532YO73eL4qpiMLVHvV7dWt2PuKGIU00lOFM0x9SYRydWLkOLWYy JHpMQ8YW0n5kKXn4VXgXHzUHUEXt6PyJBg== X-Google-Smtp-Source: ABdhPJwKcPMYzK0aqegpRFJTOQ1YkvqA/U36e8KBYv9wsw/1R5sP+MJNz65oIh6gkeAjgRFZQ1iB3w== X-Received: by 2002:adf:9e4c:: with SMTP id v12mr27788196wre.22.1606109053626; Sun, 22 Nov 2020 21:24:13 -0800 (PST) Received: from localhost.localdomain ([136.49.211.192]) by smtp.gmail.com with ESMTPSA id f16sm11631780wmh.7.2020.11.22.21.24.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 22 Nov 2020 21:24:13 -0800 (PST) From: Glenn Washburn To: grub-devel@gnu.org Cc: Patrick Steinhardt , Daniel Kiper , Glenn Washburn Subject: [PATCH v5 06/11] luks2: Better error handling when setting up the cryptodisk Date: Sun, 22 Nov 2020 23:23:19 -0600 Message-Id: <7bf56c4b3162ac6839a466c22a20a70a3a0edde4.1606108123.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=2a00:1450:4864:20::443; envelope-from=development@efficientek.com; helo=mail-wr1-x443.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: Mon, 23 Nov 2020 05:24:17 -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", 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 so long as the source disk's size is greater than this segment size. Otherwise, this is an invalid segment, and continue on to the next key. Signed-off-by: Glenn Washburn --- grub-core/disk/luks2.c | 77 ++++++++++++++++++++++++++++++++++++++++-- include/grub/disk.h | 16 +++++++++ 2 files changed, 90 insertions(+), 3 deletions(-) diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c index 5e3b255b9..2f2515fb6 100644 --- a/grub-core/disk/luks2.c +++ b/grub-core/disk/luks2.c @@ -597,12 +597,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 (i = 0; i < size; i++) { + typeof(source->total_sectors) max_crypt_sectors = 0; + + grub_errno = GRUB_ERR_NONE; ret = luks2_get_keyslot (&keyslot, &digest, &segment, json, i); 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) { @@ -616,11 +630,68 @@ 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" + " greater than source disk size, skipping\n", + segment.slot_key); + 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; + crypt->total_sectors = grub_strtoull (segment.size, NULL, 10) >> crypt->log_sector_size; + if (grub_errno == GRUB_ERR_NONE) + ; + else if(grub_errno == GRUB_ERR_BAD_NUMBER) + { + /* TODO: Unparsable number-string, try to use the whole disk */ + grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" size" + " is not a parsable number\n", + segment.slot_key); + 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" + " overflowed 64-bit unsigned integer," + " the end of the crypto device will be" + " inaccessible\n", + segment.slot_key); + 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.slot_key); + 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.slot_key); + /* 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 132a1bb75..1a9e8fcf4 100644 --- a/include/grub/disk.h +++ b/include/grub/disk.h @@ -174,6 +174,22 @@ typedef struct grub_disk_memberlist *grub_disk_memberlist_t; /* Return value of grub_disk_get_size() 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) + { + /* Round up to the nearest log_sector_size sized sector. */ + sector += 1ULL << ((log_sector_size / disk->log_sector_size) - 1); + 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