From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1kFyB9-0006SS-50 for mharc-grub-devel@gnu.org; Wed, 09 Sep 2020 07:22:19 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:58700) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kFyB7-0006SK-T2 for grub-devel@gnu.org; Wed, 09 Sep 2020 07:22:17 -0400 Received: from userp2120.oracle.com ([156.151.31.85]:33900) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kFyB5-0005pF-Eu for grub-devel@gnu.org; Wed, 09 Sep 2020 07:22:17 -0400 Received: from pps.filterd (userp2120.oracle.com [127.0.0.1]) by userp2120.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 089BKBvc101973; Wed, 9 Sep 2020 11:22:07 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=date : from : to : cc : subject : message-id : references : mime-version : content-type : in-reply-to; s=corp-2020-01-29; bh=vv+2R2ZeegstGa/3jrC2lBkbIsmojz6FAF7oj+3Ga4Q=; b=jWuZoHhBHMyb/DHWphf9IB+7NkiQ/qmgnC4W5MMsmdrYM/bZrA3FjGbODS3YgN0B2DUz /fSv9o1kWsoT2UXwPnoXgaA08/PufP/RGWPTattXyPbLnxMwHEuZyw/g/QPR/XDeuO+3 LG9dY9XoHv1cnMANxjB2ulYJRkCfYxwiELMgMvTq7IyUXC1kyBxw9guTDDp5lY7lyncT i4rM61Aty369GKjKhuzOgqEwwapAa7QPzel8TXxWyPZazrmqH1M9+tv0v4+4d5WFquwz 3o+9jXQ38LW97s4IEZAkLhQnlaA96Hhvi4uhELB9Kz7hAvN6Ipi+nPDLfiZ1oc0rT8BW vg== Received: from userp3020.oracle.com (userp3020.oracle.com [156.151.31.79]) by userp2120.oracle.com with ESMTP id 33c3an0ybk-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 09 Sep 2020 11:22:07 +0000 Received: from pps.filterd (userp3020.oracle.com [127.0.0.1]) by userp3020.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 089BJZO6084415; Wed, 9 Sep 2020 11:22:07 GMT Received: from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235]) by userp3020.oracle.com with ESMTP id 33cmespqje-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 09 Sep 2020 11:22:07 +0000 Received: from abhmp0010.oracle.com (abhmp0010.oracle.com [141.146.116.16]) by aserv0121.oracle.com (8.14.4/8.13.8) with ESMTP id 089BM0q0011537; Wed, 9 Sep 2020 11:22:06 GMT Received: from tomti.i.net-space.pl (/10.175.189.45) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 09 Sep 2020 04:21:59 -0700 Date: Wed, 9 Sep 2020 13:21:55 +0200 From: Daniel Kiper To: Patrick Steinhardt Cc: grub-devel@gnu.org, Denis GNUtoo Carikli , Glenn Washburn Subject: Re: [PATCH v3 9/9] cryptodisk: Properly handle non-512 byte sized sectors Message-ID: <20200909112155.bqrrcazvb73lrtsy@tomti.i.net-space.pl> References: <81b8a7f915ff1a4499bd9c2e2ca92828a887d046.1599492346.git.ps@pks.im> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <81b8a7f915ff1a4499bd9c2e2ca92828a887d046.1599492346.git.ps@pks.im> User-Agent: NeoMutt/20170113 (1.7.2) X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9738 signatures=668679 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 suspectscore=2 adultscore=0 bulkscore=0 phishscore=0 malwarescore=0 mlxlogscore=999 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2006250000 definitions=main-2009090101 X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9738 signatures=668679 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 priorityscore=1501 clxscore=1015 bulkscore=0 malwarescore=0 lowpriorityscore=0 mlxlogscore=999 suspectscore=2 adultscore=0 mlxscore=0 impostorscore=0 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2006250000 definitions=main-2009090101 Received-SPF: pass client-ip=156.151.31.85; envelope-from=daniel.kiper@oracle.com; helo=userp2120.oracle.com X-detected-operating-system: by eggs.gnu.org: First seen = 2020/09/09 07:22:14 X-ACL-Warn: Detected OS = Linux 3.1-3.10 [fuzzy] X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, UNPARSEABLE_RELAY=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: Wed, 09 Sep 2020 11:22:18 -0000 On Mon, Sep 07, 2020 at 05:28:08PM +0200, Patrick Steinhardt wrote: > From: Glenn Washburn > > By default, dm-crypt internally uses an IV that corresponds to 512-byte > sectors, even when a larger sector size is specified. What this means is > that when using a larger sector size, the IV is incremented every sector. > However, the amount the IV is incremented is the number of 512 byte blocks > in a sector (ie 8 for 4K sectors). Confusingly the IV does not corespond to > the number of, for example, 4K sectors. So each cipher block in the fifth > 4K sector will be encrypted with an IV equal to 32, as opposed to 32-39 for s/32-39/32-9/? > each sequential 512 byte block or an IV of 4 for each cipher block in the > sector. > > There are some encryption utilities which do it the intuitive way and have > the IV equal to the sector number regardless of sector size (ie. the fifth > sector would have an IV of 4 for each cipher block). And this is supported > by dm-crypt with the iv_large_sectors option and also cryptsetup as of 2.3.3 > with the --iv-large-sectors, though not with LUKS headers (only with --type > plain). However, support for this has not been included as grub does not > support plain devices right now. > > One gotcha here is that the encrypted split keys are encrypted with a hard- > coded 512-byte sector size. So even if your data is encrypted with 4K sector > sizes, the split key encrypted area must be decrypted with a block size of > 512 (ie the IV increments every 512 bytes). This made these changes less > aestetically pleasing than desired. > > Signed-off-by: Glenn Washburn > Reviewed-by: Patrick Steinhardt > --- > grub-core/disk/cryptodisk.c | 44 ++++++++++++++++++++----------------- > grub-core/disk/luks.c | 5 +++-- > grub-core/disk/luks2.c | 7 +++++- > include/grub/cryptodisk.h | 9 +++++++- > 4 files changed, 41 insertions(+), 24 deletions(-) > > diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c > index 0b63b7d96..6319f3164 100644 > --- a/grub-core/disk/cryptodisk.c > +++ b/grub-core/disk/cryptodisk.c > @@ -224,7 +224,8 @@ lrw_xor (const struct lrw_sector *sec, > static gcry_err_code_t > grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev, > grub_uint8_t * data, grub_size_t len, > - grub_disk_addr_t sector, int do_encrypt) > + grub_disk_addr_t sector, grub_size_t log_sector_size, > + int do_encrypt) > { > grub_size_t i; > gcry_err_code_t err; > @@ -237,12 +238,13 @@ grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev, > return (do_encrypt ? grub_crypto_ecb_encrypt (dev->cipher, data, data, len) > : grub_crypto_ecb_decrypt (dev->cipher, data, data, len)); > > - for (i = 0; i < len; i += (1U << dev->log_sector_size)) > + for (i = 0; i < len; i += (1U << log_sector_size)) > { > grub_size_t sz = ((dev->cipher->cipher->blocksize > + sizeof (grub_uint32_t) - 1) > / sizeof (grub_uint32_t)); > grub_uint32_t iv[(GRUB_CRYPTO_MAX_CIPHER_BLOCKSIZE + 3) / 4]; > + grub_uint64_t iv_calc = 0; > > if (dev->rekey) > { > @@ -270,7 +272,7 @@ grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev, > if (!ctx) > return GPG_ERR_OUT_OF_MEMORY; > > - tmp = grub_cpu_to_le64 (sector << dev->log_sector_size); > + tmp = grub_cpu_to_le64 (sector << log_sector_size); > dev->iv_hash->init (ctx); > dev->iv_hash->write (ctx, dev->iv_prefix, dev->iv_prefix_len); > dev->iv_hash->write (ctx, &tmp, sizeof (tmp)); > @@ -281,14 +283,16 @@ grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev, > } > break; > case GRUB_CRYPTODISK_MODE_IV_PLAIN64: > - iv[1] = grub_cpu_to_le32 (sector >> 32); > + iv_calc = sector << (log_sector_size - GRUB_CRYPTODISK_IV_LOG_SIZE); > + iv[1] = grub_cpu_to_le32 (iv_calc >> 32); Why 32? Could you use a constant or add a comment here? > /* FALLTHROUGH */ > case GRUB_CRYPTODISK_MODE_IV_PLAIN: > - iv[0] = grub_cpu_to_le32 (sector & 0xFFFFFFFF); > + iv_calc = sector << (log_sector_size - GRUB_CRYPTODISK_IV_LOG_SIZE); > + iv[0] = grub_cpu_to_le32 (iv_calc & 0xFFFFFFFF); > break; > case GRUB_CRYPTODISK_MODE_IV_BYTECOUNT64: > - iv[1] = grub_cpu_to_le32 (sector >> (32 - dev->log_sector_size)); Ditto? > - iv[0] = grub_cpu_to_le32 ((sector << dev->log_sector_size) > + iv[1] = grub_cpu_to_le32 (sector >> (32 - log_sector_size)); Ditto? [...] > diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c > index 59702067a..2e1347b13 100644 > --- a/grub-core/disk/luks.c > +++ b/grub-core/disk/luks.c > @@ -124,7 +124,7 @@ configure_ciphers (grub_disk_t disk, const char *check_uuid, > return NULL; > newdev->offset = grub_be_to_cpu32 (header.payloadOffset); > newdev->source_disk = NULL; > - newdev->log_sector_size = 9; > + newdev->log_sector_size = LUKS_LOG_SECTOR_SIZE; > newdev->total_length = grub_disk_get_size (disk) - newdev->offset; > grub_memcpy (newdev->uuid, uuid, sizeof (uuid)); > newdev->modname = "luks"; > @@ -247,7 +247,8 @@ luks_recover_key (grub_disk_t source, > return err; > } > > - gcry_err = grub_cryptodisk_decrypt (dev, split_key, length, 0); > + gcry_err = grub_cryptodisk_decrypt (dev, split_key, length, 0, > + LUKS_LOG_SECTOR_SIZE); > if (gcry_err) > { > grub_free (split_key); > diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c > index 26e1126b1..eb64a0596 100644 > --- a/grub-core/disk/luks2.c > +++ b/grub-core/disk/luks2.c > @@ -492,7 +492,12 @@ luks2_decrypt_key (grub_uint8_t *out_key, > goto err; > } > > - gcry_ret = grub_cryptodisk_decrypt (crypt, split_key, k->area.size, 0); > + /* > + * The key slots area is always encrypted in 512-byte sectors, > + * regardless of encrypted data sector size. > + */ > + gcry_ret = grub_cryptodisk_decrypt (crypt, split_key, k->area.size, 0, > + LUKS_LOG_SECTOR_SIZE); s/LUKS_LOG_SECTOR_SIZE/GRUB_CRYPTODISK_IV_LOG_SIZE/? > if (gcry_ret) > { > ret = grub_crypto_gcry_error (gcry_ret); > diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h > index e1b21e785..ecb37ba43 100644 > --- a/include/grub/cryptodisk.h > +++ b/include/grub/cryptodisk.h > @@ -48,6 +48,13 @@ typedef enum > > #define GRUB_CRYPTODISK_MAX_UUID_LENGTH 71 > > +#define LUKS_LOG_SECTOR_SIZE 9 > + > +/* For the purposes of IV incrementing the sector size is 512 bytes, unless > + * otherwise specified. > + */ > +#define GRUB_CRYPTODISK_IV_LOG_SIZE 9 > + > #define GRUB_CRYPTODISK_GF_LOG_SIZE 7 > #define GRUB_CRYPTODISK_GF_SIZE (1U << GRUB_CRYPTODISK_GF_LOG_SIZE) > #define GRUB_CRYPTODISK_GF_LOG_BYTES (GRUB_CRYPTODISK_GF_LOG_SIZE - 3) > @@ -139,7 +146,7 @@ grub_cryptodisk_setkey (grub_cryptodisk_t dev, > gcry_err_code_t > grub_cryptodisk_decrypt (struct grub_cryptodisk *dev, > grub_uint8_t * data, grub_size_t len, > - grub_disk_addr_t sector); > + grub_disk_addr_t sector, grub_size_t log_sector_size); > grub_err_t > grub_cryptodisk_insert (grub_cryptodisk_t newdev, const char *name, > grub_disk_t source); Daniel