From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1kUeKU-0000UV-1M for mharc-grub-devel@gnu.org; Mon, 19 Oct 2020 19:12:39 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:40608) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kUeKR-0000UK-JZ for grub-devel@gnu.org; Mon, 19 Oct 2020 19:12:35 -0400 Received: from mail-oi1-x22f.google.com ([2607:f8b0:4864:20::22f]:35668) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kUeKO-0007Tk-6B for grub-devel@gnu.org; Mon, 19 Oct 2020 19:12:35 -0400 Received: by mail-oi1-x22f.google.com with SMTP id w141so5113oia.2 for ; Mon, 19 Oct 2020 16:12:31 -0700 (PDT) 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=7eXnO0NLD1fiSViSY42fUbH+mNCck74lXKHGXFvp0Dw=; b=H3d6lewI1DW6s/bVfvLVwcwO16JqqJZhpqPInCEzqAvIJQuzKkWHbND7ISr0YczwNS CTW0Mz0gp9PNxtEniOjZt3OnjUtQODIHlHOLZ+DEP7VA8297ELTkhC5Va0TUDJQnbbGS jMUcaK0dhrLDBMTtdfclNhUlDxXRQyXC6mAO00B3xeVzI4CQUdcCN9Mp1XYQQklDJpKC wTN/Lp64x9ubwnUAzPF0hPZO2/epWQXbgIePmlfJHTQ6CD6gPBKGJD/ap4g3mXii4vpj xI+lAtvJs0qBdjge+dfqL9NjSXsu59KODfLX+GTZRBNARpcvi2VPTDfVxIsDRLFU8hkN qusw== 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=7eXnO0NLD1fiSViSY42fUbH+mNCck74lXKHGXFvp0Dw=; b=Bd//0wwVxq/xCH0as5f4Hw+hD+5B5rxeL+Keo4hR+S1oklf+8E85Bers6jVZKOW/wI fLHvojIpG4+/JJlIncDlr8bDDjW7GYvVJ+L198m9x0A07gdUVViDQiiBMgO82eWbANqO acdD3jdTfgMn9VMTeKRyOgjVJ0Vs5VEWEx3ZHYKch15go5o4de23MCKhNgFi/GvvVsS7 9FXwEqLqcVX+nLnvdWRxwMEYR5GhF724PbdtKgVWUz0Wj9zt8ghPo6dOdU2Z5dY7rsRE dY96aqRRIedQPv1riOkmMuFmcz+j80tMGdOrbUbmO2RKuMIEnrClzeU6P5bISW152l8X CuDQ== X-Gm-Message-State: AOAM533B959k+gqjAwwCBAlCohLaLNwY8UYCuqc+JnbC3FVn7CxZUJh0 x8pv86+42T2vpOidQL+s/8OTFZ4BVwzjdw== X-Google-Smtp-Source: ABdhPJzLFzpjZFBrmy3GhxOLdgwFoOGeCmpjtandoSDztb6VARGK+Zx29/jnqs8lsaZYXvVP32pusA== X-Received: by 2002:aca:5347:: with SMTP id h68mr91251oib.161.1603149149606; Mon, 19 Oct 2020 16:12:29 -0700 (PDT) Received: from localhost.localdomain (47-218-232-180.bcstcmtk03.res.dyn.suddenlink.net. [47.218.232.180]) by smtp.gmail.com with ESMTPSA id d27sm310848otc.6.2020.10.19.16.12.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 19 Oct 2020 16:12:28 -0700 (PDT) From: Glenn Washburn To: grub-devel@gnu.org Cc: Patrick Steinhardt , Daniel Kiper , Glenn Washburn Subject: [PATCH v3 00/10] Cryptodisk fixes for v2.06 redux Date: Mon, 19 Oct 2020 18:09:48 -0500 Message-Id: X-Mailer: git-send-email 2.27.0 In-Reply-To: <20201009100122.GH2088@tanuki> References: <20201009100122.GH2088@tanuki> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Received-SPF: pass client-ip=2607:f8b0:4864:20::22f; envelope-from=development@efficientek.com; helo=mail-oi1-x22f.google.com X-detected-operating-system: by eggs.gnu.org: No matching host in p0f cache. That's all we know. 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, 19 Oct 2020 23:12:35 -0000 Heres an updated patch series which addresses comment from Patrick. The only code change is adding a slot_key member to grub_luks2_keyslot and using that instead of an extra out parameter to luks2_get_keyslot. Glenn Washburn (10): luks2: Fix use of incorrect index and some grub_error() messages. luks2: Improve readability in luks2_get_keyslot. luks2: Use more intuitive keyslot key instead of index when naming keyslot. luks2: grub_cryptodisk_t->total_length is the max number of device native sectors cryptodisk: Fix cipher IV mode 'plain64' always being set as 'plain'. cryptodisk: Properly handle non-512 byte sized sectors. cryptodisk: Replace some literals with constants in grub_cryptodisk_endecrypt. cryptodisk: Rename total_length field in grub_cryptodisk_t to total_sectors. cryptodisk: Rename offset in grub_cryptodisk_t to offset_sectors. luks2: Rename source disk variabled named 'disk' to 'source' as in luks.c. grub-core/disk/cryptodisk.c | 78 +++++++++++++++++++-------------- grub-core/disk/geli.c | 4 +- grub-core/disk/luks.c | 9 ++-- grub-core/disk/luks2.c | 86 ++++++++++++++++++++----------------- include/grub/cryptodisk.h | 18 ++++++-- include/grub/types.h | 3 ++ 6 files changed, 117 insertions(+), 81 deletions(-) Range-diff against v2: 1: e2433b8ab ! 1: 1f65a04e0 luks2: Use more intuitive keyslot key instead of index when naming keyslot. @@ Commit message cryptsetup. ## grub-core/disk/luks2.c ## -@@ grub-core/disk/luks2.c: luks2_parse_digest (grub_luks2_digest_t *out, const grub_json_t *digest) +@@ grub-core/disk/luks2.c: typedef struct grub_luks2_header grub_luks2_header_t; - static grub_err_t - luks2_get_keyslot (grub_luks2_keyslot_t *k, grub_luks2_digest_t *d, grub_luks2_segment_t *s, -- const grub_json_t *root, grub_size_t keyslot_idx) -+ grub_uint64_t *keyslot_key, const grub_json_t *root, grub_size_t keyslot_idx) + struct grub_luks2_keyslot + { ++ grub_uint64_t slot_key; + grub_int64_t key_size; + grub_int64_t priority; + struct +@@ grub-core/disk/luks2.c: luks2_get_keyslot (grub_luks2_keyslot_t *k, grub_luks2_digest_t *d, grub_luks2_s { grub_json_t keyslots, keyslot, digests, digest, segments, segment; grub_size_t i, size; @@ grub-core/disk/luks2.c: luks2_parse_digest (grub_luks2_digest_t *out, const grub if (grub_json_getvalue (&keyslots, root, "keyslots") || grub_json_getchild (&keyslot, &keyslots, keyslot_idx) || - grub_json_getuint64 (&keyslot_key, &keyslot, NULL) || -+ grub_json_getuint64 (keyslot_key, &keyslot, NULL) || ++ grub_json_getuint64 (&k->slot_key, &keyslot, NULL) || grub_json_getchild (&keyslot, &keyslot, 0) || luks2_parse_keyslot (k, &keyslot)) return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse keyslot index %"PRIuGRUB_SIZE, keyslot_idx); @@ grub-core/disk/luks2.c: luks2_get_keyslot (grub_luks2_keyslot_t *k, grub_luks2_d return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse digest index %"PRIuGRUB_SIZE, i); - if ((d->keyslots & (1 << keyslot_key))) -+ if ((d->keyslots & (1 << *keyslot_key))) ++ if ((d->keyslots & (1 << k->slot_key))) break; } if (i == size) - return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot \"%"PRIuGRUB_UINT64_T"\"", keyslot_key); -+ return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot \"%"PRIuGRUB_UINT64_T"\"", *keyslot_key); ++ return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot \"%"PRIuGRUB_UINT64_T"\"", k->slot_key); /* Get segment that matches the digest. */ if (grub_json_getvalue (&segments, root, "segments") || @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t disk, - /* Try all keyslot */ - for (i = 0; i < size; i++) - { -- ret = luks2_get_keyslot (&keyslot, &digest, &segment, json, i); -+ grub_uint64_t keyslot_key; -+ ret = luks2_get_keyslot (&keyslot, &digest, &segment, &keyslot_key, json, i); - if (ret) - goto err; if (keyslot.priority == 0) { - grub_dprintf ("luks2", "Ignoring keyslot %"PRIuGRUB_SIZE" due to priority\n", i); -+ grub_dprintf ("luks2", "Ignoring keyslot %"PRIuGRUB_UINT64_T" due to priority\n", keyslot_key); ++ grub_dprintf ("luks2", "Ignoring keyslot %"PRIuGRUB_UINT64_T" due to priority\n", keyslot.slot_key); continue; } - grub_dprintf ("luks2", "Trying keyslot %"PRIuGRUB_SIZE"\n", i); -+ grub_dprintf ("luks2", "Trying keyslot %"PRIuGRUB_UINT64_T"\n", keyslot_key); ++ grub_dprintf ("luks2", "Trying keyslot %"PRIuGRUB_UINT64_T"\n", keyslot.slot_key); /* Set up disk according to keyslot's segment. */ crypt->offset = grub_divmod64 (segment.offset, segment.sector_size, NULL); @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t disk, - grub_dprintf ("luks2", "Decryption with keyslot %"PRIuGRUB_SIZE" failed: %s\n", - i, grub_errmsg); + grub_dprintf ("luks2", "Decryption with keyslot %"PRIuGRUB_UINT64_T" failed: %s\n", -+ keyslot_key, grub_errmsg); ++ keyslot.slot_key, grub_errmsg); continue; } @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t disk, - grub_dprintf ("luks2", "Could not open keyslot %"PRIuGRUB_SIZE": %s\n", - i, grub_errmsg); + grub_dprintf ("luks2", "Could not open keyslot %"PRIuGRUB_UINT64_T": %s\n", -+ keyslot_key, grub_errmsg); ++ keyslot.slot_key, grub_errmsg); continue; } @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t disk, * where each element is either empty or holds a key. */ - grub_printf_ (N_("Slot %"PRIuGRUB_SIZE" opened\n"), i); -+ grub_printf_ (N_("Slot %"PRIuGRUB_UINT64_T" opened\n"), keyslot_key); ++ grub_printf_ (N_("Slot %"PRIuGRUB_UINT64_T" opened\n"), keyslot.slot_key); candidate_key_len = keyslot.key_size; break; 2: 3baffdd4f ! 2: fcb72f70d luks2: grub_cryptodisk_t->total_length is the max number of device native sectors @@ Commit message The total_length field is named confusingly because length usually refers to bytes, whereas in this case its really the total number of sectors on the device. Also counter-intuitively, grub_disk_get_size returns the total - number of device native sectors sectors. We need to convert the sectors from - the size of the underlying device to the cryptodisk sector size. And + number of device native sectors. We need to convert the sectors from the + size of the underlying device to the cryptodisk sector size. And segment.size is in bytes which need to be converted to cryptodisk sectors. Also, removed an empty statement. 3: 6da3d8598 = 3: e8e069cae cryptodisk: Fix cipher IV mode 'plain64' always being set as 'plain'. 4: fd7cb6b16 = 4: 6fe26ba56 cryptodisk: Properly handle non-512 byte sized sectors. 5: b33733199 = 5: 3918a9013 cryptodisk: Replace some literals with constants in grub_cryptodisk_endecrypt. 6: de4f6b2e5 ! 6: 28e0cac66 cryptodisk: Rename total_length field in grub_cryptodisk_t to total_sectors. @@ Metadata ## Commit message ## cryptodisk: Rename total_length field in grub_cryptodisk_t to total_sectors. - This makes the creates an alignment with grub_disk_t naming of the same - field and is more intuitive as to how it should be used. + This creates an alignment with grub_disk_t naming of the same field and is + more intuitive as to how it should be used. ## grub-core/disk/cryptodisk.c ## @@ grub-core/disk/cryptodisk.c: grub_cryptodisk_open (const char *name, grub_disk_t disk) 7: a165791de ! 7: 74c6232c9 cryptodisk: Rename offset in grub_cryptodisk_t to offset_sectors. @@ grub-core/disk/luks.c: configure_ciphers (grub_disk_t disk, const char *check_uu ## grub-core/disk/luks2.c ## @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t disk, - grub_dprintf ("luks2", "Trying keyslot %"PRIuGRUB_UINT64_T"\n", keyslot_key); + grub_dprintf ("luks2", "Trying keyslot %"PRIuGRUB_UINT64_T"\n", keyslot.slot_key); /* Set up disk according to keyslot's segment. */ - crypt->offset = grub_divmod64 (segment.offset, segment.sector_size, NULL); 8: 86beb5be8 = 8: 738fe0139 luks2: Rename source disk variabled named 'disk' to 'source' as in luks.c. -- 2.27.0