From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1kiZgR-0004xf-5K for mharc-grub-devel@gnu.org; Fri, 27 Nov 2020 04:04:51 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:33008) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kiZgN-0004vR-Ry for grub-devel@gnu.org; Fri, 27 Nov 2020 04:04:48 -0500 Received: from mail-lj1-x241.google.com ([2a00:1450:4864:20::241]:34448) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kiZgJ-0005Vr-Su for grub-devel@gnu.org; Fri, 27 Nov 2020 04:04:46 -0500 Received: by mail-lj1-x241.google.com with SMTP id y16so5163611ljk.1 for ; Fri, 27 Nov 2020 01:04:43 -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=t2nVZmHYbPlO+4VrmA7T8m2q8eNsxF3NAO+aw/L3rz0=; b=oTN/BY46qZ1mLst/3hh1Ire1eIqxDm9jiVKjQgmM7QLvhWsJIu/P67mByb0hO4s4w5 zgZToGfNu8C55tXUx7zEOYRP5MWJFtPI5LLJYg3IAL5RTotXbG6PElcXi777W8Az74LV V/BL7fzIiGbPp59Ajl6Kzzdl/EZ0dzo+kHV0McrZEUXuNzOcqrBcOHOAyj4ufeX2Yjiv YBLZVQrcXgNI5X+1WhuFLvmnaAnv9whPwoonXh5ilk79M+ueMd2T3eGUs8grtYHncjSL 0Wiqe6zdtd2RJeunh9Ykma6hgsYwRfoCFr9FsOd4YPfpQgdZk1WA6fhc/5X1/tYieww/ ejIA== 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=t2nVZmHYbPlO+4VrmA7T8m2q8eNsxF3NAO+aw/L3rz0=; b=sU3NWQvG688pphS3ZQ4tcMN7w2ff+YWRNa/mh+ebariexmeGOyDYClnyCLGbjIowAD XWHj8Vj1R/rB1ay1bUU3blUSGMczmTDDWs8gvCE3t0u2XnePYuD/HGb1k4FhFApcvW+P HoioRN4aifi6hxmCIjOT1Lx5gsOX6Q4XZdqT84TjRrwaVRGd49StC6r79bQBrnP5omEm 3CO0W7xSGM6oqc0yCtbObUkH/FKIdKfTzEe7bj1IiZ+OFqogOAXKiKjnT87J9b8a6wcI kt4HQgXq7ZIVzCncDwllcN7VpTT8GmVacUsxOzvJSuNMg6+uOrFEpQLfoVys69Zt4L1r /gEA== X-Gm-Message-State: AOAM530/iGLA75IPpYb7VCu+X3V5IXEdkzvfaBElpewnDNwFr4B9+ghL 5rg045N3384n5VhKtZyKg3TZzsJyLGFWwA== X-Google-Smtp-Source: ABdhPJxhIf7L4xlo7KbMsWY2MgxJ7nxIEw6erhGnLvGMvTliELyeDAEUnp2xlSU1YehoITaY6i+B2w== X-Received: by 2002:a05:651c:1246:: with SMTP id h6mr3049653ljh.187.1606467881719; Fri, 27 Nov 2020 01:04:41 -0800 (PST) Received: from localhost.localdomain ([136.49.211.192]) by smtp.gmail.com with ESMTPSA id g2sm880675lja.87.2020.11.27.01.04.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 27 Nov 2020 01:04:41 -0800 (PST) From: Glenn Washburn To: grub-devel@gnu.org Cc: Patrick Steinhardt , Daniel Kiper , Glenn Washburn Subject: [PATCH v6 07/12] luks2: Better error handling when setting up the cryptodisk Date: Fri, 27 Nov 2020 03:03:39 -0600 Message-Id: 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::241; envelope-from=development@efficientek.com; helo=mail-lj1-x241.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: Fri, 27 Nov 2020 09:04:48 -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 | 80 ++++++++++++++++++++++++++++++++++++++++-- include/grub/disk.h | 16 +++++++++ 2 files changed, 93 insertions(+), 3 deletions(-) diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c index b7ed0642e..01f9608e5 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,71 @@ 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.slot_key, 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; + 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" + " \"%s\" is not a parsable number\n", + segment.slot_key, 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.slot_key, 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.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