From: Christoph Hellwig <hch@infradead.org> To: Eric Biggers <ebiggers@kernel.org> Cc: Christoph Hellwig <hch@infradead.org>, "Martin K. Petersen" <martin.petersen@oracle.com>, "Darrick J. Wong" <darrick.wong@oracle.com>, Satya Tangirala <satyat@google.com>, linux-block@vger.kernel.org, linux-scsi@vger.kernel.org, linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, Barani Muthukumaran <bmuthuku@qti.qualcomm.com>, Kuohong Wang <kuohong.wang@mediatek.com>, Kim Boojin <boojin.kim@samsung.com> Subject: Re: [PATCH v6 2/9] block: Add encryption context to struct bio Date: Fri, 17 Jan 2020 00:32:21 -0800 [thread overview] Message-ID: <20200117083221.GA324@infradead.org> (raw) In-Reply-To: <20200108172629.GA232722@sol.localdomain> Hi Eric, On Wed, Jan 08, 2020 at 09:26:29AM -0800, Eric Biggers wrote: > The NVMe "key per I/O" draft is heavily flawed, and I don't think it will be > useful at all in the Linux kernel context. The problem is that, as far as I > can tell, it doesn't allow the encryption algorithm and IVs to be selected, > or even standardized or made discoverable in any way. It does say that > AES-256 must be supported, but it doesn't say which mode of operation (i.e. > it could be something inappropriate for disk encryption, like ECB), nor > does it say whether AES-256 has to be the default or not, and if it's not > the default how to discover that and select AES-256. I've talked to people involved with the TCG side of this spec, where all the interesting crypto happens. Currently the plan is to support KMIP wrapper keys, which specify the exact algorithm and operation mode, and algorithms and modes for the initial version are planned to be AES 256/512 XTS. I also had a chat with an involved person and they understand the principle that for the inline crypto to be trusted it needs to be interoperable with (trusted) software algorithms. So I don't think it is all doom. > IV generation is also unspecified, so it > could be something insecure like always using the same IV. From talking to one of the initiators of the spec, no it is not intended to be unspecified, but indeed tied to the LBA (see below). > Also, since "key per I/O" won't allow selecting IVs, all the encrypted data will > be tied to its physical location on-disk. That will make "key per I/O" unusable > in any case where encrypted blocks are moved without the key, e.g. > filesystem-level encryption on many filesystems. File systems don't move data around all that often (saying that with my fs developer hat on). In traditional file systems only defragmentation will move data around, with extent refcounting it can also happen for dedup, and for file systems that write out of place data gets moved when parts of a block are rewritten, but in that case a read modify write cycle is perfomed in the Linux code anyway, so it will go through the inline encryption engined on the way and the way out. So in other words - specifying an IV would be useful for some use cases, but I don't think it is a deal blocker. Even without that is is useful for block device level encryption, and could have some usefulness for file system encryption usage. I think that adding an IV would eventually be useful, but fitting that into NVMe won't be easy, as you'd need to find a way to specify the IV for each submission queue entry, which requires growing it, or finding some way to extend it out of band. > I've already raised these concerns in the NVMe and TCG Storage working groups, > and the people working on it refused to make any changes, as they consider "key > per I/O" to be more akin to the TCG Opal self-encrypting drive specification, > and not actually intended to be "inline encryption". While I have my fair share of issues how the spec is developed that isn't my impression, and at least for the verifyable part I heard contrary statements. Feel free to contact me offline to make sure we can move this into the right direction. > So let's not over-engineer this kernel patchset to support some broken > vaporware, please. Not sharing bio fields for integrity and encryption actually keeps the patchset simpler (although uses more memory if both options are enabled). So my main point here is to not over engineer it for broken premise that won't be true soon.
WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org> To: Eric Biggers <ebiggers@kernel.org> Cc: linux-block@vger.kernel.org, "Martin K. Petersen" <martin.petersen@oracle.com>, linux-scsi@vger.kernel.org, "Darrick J. Wong" <darrick.wong@oracle.com>, Kuohong Wang <kuohong.wang@mediatek.com>, Kim Boojin <boojin.kim@samsung.com>, Barani Muthukumaran <bmuthuku@qti.qualcomm.com>, Satya Tangirala <satyat@google.com>, Christoph Hellwig <hch@infradead.org>, linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net Subject: Re: [f2fs-dev] [PATCH v6 2/9] block: Add encryption context to struct bio Date: Fri, 17 Jan 2020 00:32:21 -0800 [thread overview] Message-ID: <20200117083221.GA324@infradead.org> (raw) In-Reply-To: <20200108172629.GA232722@sol.localdomain> Hi Eric, On Wed, Jan 08, 2020 at 09:26:29AM -0800, Eric Biggers wrote: > The NVMe "key per I/O" draft is heavily flawed, and I don't think it will be > useful at all in the Linux kernel context. The problem is that, as far as I > can tell, it doesn't allow the encryption algorithm and IVs to be selected, > or even standardized or made discoverable in any way. It does say that > AES-256 must be supported, but it doesn't say which mode of operation (i.e. > it could be something inappropriate for disk encryption, like ECB), nor > does it say whether AES-256 has to be the default or not, and if it's not > the default how to discover that and select AES-256. I've talked to people involved with the TCG side of this spec, where all the interesting crypto happens. Currently the plan is to support KMIP wrapper keys, which specify the exact algorithm and operation mode, and algorithms and modes for the initial version are planned to be AES 256/512 XTS. I also had a chat with an involved person and they understand the principle that for the inline crypto to be trusted it needs to be interoperable with (trusted) software algorithms. So I don't think it is all doom. > IV generation is also unspecified, so it > could be something insecure like always using the same IV. From talking to one of the initiators of the spec, no it is not intended to be unspecified, but indeed tied to the LBA (see below). > Also, since "key per I/O" won't allow selecting IVs, all the encrypted data will > be tied to its physical location on-disk. That will make "key per I/O" unusable > in any case where encrypted blocks are moved without the key, e.g. > filesystem-level encryption on many filesystems. File systems don't move data around all that often (saying that with my fs developer hat on). In traditional file systems only defragmentation will move data around, with extent refcounting it can also happen for dedup, and for file systems that write out of place data gets moved when parts of a block are rewritten, but in that case a read modify write cycle is perfomed in the Linux code anyway, so it will go through the inline encryption engined on the way and the way out. So in other words - specifying an IV would be useful for some use cases, but I don't think it is a deal blocker. Even without that is is useful for block device level encryption, and could have some usefulness for file system encryption usage. I think that adding an IV would eventually be useful, but fitting that into NVMe won't be easy, as you'd need to find a way to specify the IV for each submission queue entry, which requires growing it, or finding some way to extend it out of band. > I've already raised these concerns in the NVMe and TCG Storage working groups, > and the people working on it refused to make any changes, as they consider "key > per I/O" to be more akin to the TCG Opal self-encrypting drive specification, > and not actually intended to be "inline encryption". While I have my fair share of issues how the spec is developed that isn't my impression, and at least for the verifyable part I heard contrary statements. Feel free to contact me offline to make sure we can move this into the right direction. > So let's not over-engineer this kernel patchset to support some broken > vaporware, please. Not sharing bio fields for integrity and encryption actually keeps the patchset simpler (although uses more memory if both options are enabled). So my main point here is to not over engineer it for broken premise that won't be true soon. _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next prev parent reply other threads:[~2020-01-17 8:32 UTC|newest] Thread overview: 112+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-18 14:51 [PATCH v6 0/9] Inline Encryption Support Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-18 14:51 ` [PATCH v6 1/9] block: Keyslot Manager for Inline Encryption Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-18 20:13 ` Eric Biggers 2019-12-18 20:13 ` [f2fs-dev] " Eric Biggers 2020-01-17 9:10 ` Christoph Hellwig 2020-01-17 9:10 ` [f2fs-dev] " Christoph Hellwig 2019-12-18 14:51 ` [PATCH v6 2/9] block: Add encryption context to struct bio Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-18 21:10 ` Eric Biggers 2019-12-18 21:10 ` [f2fs-dev] " Eric Biggers 2019-12-18 21:21 ` Darrick J. Wong 2019-12-18 21:21 ` [f2fs-dev] " Darrick J. Wong 2019-12-18 21:25 ` Martin K. Petersen 2019-12-18 21:25 ` [f2fs-dev] " Martin K. Petersen 2019-12-18 22:27 ` Eric Biggers 2019-12-18 22:27 ` [f2fs-dev] " Eric Biggers 2019-12-19 0:47 ` Martin K. Petersen 2019-12-19 0:47 ` [f2fs-dev] " Martin K. Petersen 2019-12-20 3:52 ` Eric Biggers 2019-12-20 3:52 ` [f2fs-dev] " Eric Biggers 2020-01-07 4:35 ` Martin K. Petersen 2020-01-07 4:35 ` [f2fs-dev] " Martin K. Petersen 2020-01-08 14:07 ` Christoph Hellwig 2020-01-08 14:07 ` [f2fs-dev] " Christoph Hellwig 2020-01-08 17:26 ` Eric Biggers 2020-01-08 17:26 ` [f2fs-dev] " Eric Biggers 2020-01-17 8:32 ` Christoph Hellwig [this message] 2020-01-17 8:32 ` Christoph Hellwig 2020-01-18 5:11 ` Eric Biggers 2020-01-18 5:11 ` [f2fs-dev] " Eric Biggers 2020-01-21 22:05 ` Satya Tangirala 2020-01-21 22:05 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-01-09 3:40 ` Martin K. Petersen 2020-01-09 3:40 ` [f2fs-dev] " Martin K. Petersen 2020-01-14 21:24 ` Eric Biggers 2020-01-14 21:24 ` [f2fs-dev] " Eric Biggers 2019-12-18 14:51 ` [PATCH v6 3/9] block: blk-crypto for Inline Encryption Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-20 3:14 ` Eric Biggers 2019-12-20 3:14 ` [f2fs-dev] " Eric Biggers 2019-12-20 5:10 ` Eric Biggers 2019-12-20 5:10 ` [f2fs-dev] " Eric Biggers 2020-01-14 21:22 ` Eric Biggers 2020-01-14 21:22 ` [f2fs-dev] " Eric Biggers 2019-12-18 14:51 ` [PATCH v6 4/9] scsi: ufs: UFS driver v2.1 spec crypto additions Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-01-17 12:31 ` Christoph Hellwig 2020-01-17 12:31 ` [f2fs-dev] " Christoph Hellwig 2019-12-18 14:51 ` [PATCH v6 5/9] scsi: ufs: UFS crypto API Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-20 4:48 ` Eric Biggers 2019-12-20 4:48 ` [f2fs-dev] " Eric Biggers 2020-01-14 21:16 ` Eric Biggers 2020-01-14 21:16 ` [f2fs-dev] " Eric Biggers 2020-01-17 13:51 ` Christoph Hellwig 2020-01-17 13:51 ` [f2fs-dev] " Christoph Hellwig 2019-12-18 14:51 ` [PATCH v6 6/9] scsi: ufs: Add inline encryption support to UFS Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-20 5:44 ` Eric Biggers 2019-12-20 5:44 ` [f2fs-dev] " Eric Biggers 2020-01-17 13:58 ` Christoph Hellwig 2020-01-17 13:58 ` [f2fs-dev] " Christoph Hellwig 2020-01-18 5:27 ` Eric Biggers 2020-01-18 5:27 ` [f2fs-dev] " Eric Biggers 2020-02-05 18:07 ` Christoph Hellwig 2020-02-05 18:07 ` [f2fs-dev] " Christoph Hellwig 2020-01-18 3:58 ` Eric Biggers 2020-01-18 3:58 ` [f2fs-dev] " Eric Biggers 2020-02-05 20:47 ` Eric Biggers 2020-02-05 20:47 ` [f2fs-dev] " Eric Biggers 2019-12-18 14:51 ` [PATCH v6 7/9] fscrypt: add inline encryption support Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-01-14 21:12 ` Eric Biggers 2020-01-14 21:12 ` [f2fs-dev] " Eric Biggers 2019-12-18 14:51 ` [PATCH v6 8/9] f2fs: " Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-20 4:23 ` Eric Biggers 2019-12-20 4:23 ` [f2fs-dev] " Eric Biggers 2019-12-18 14:51 ` [PATCH v6 9/9] ext4: " Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-19 0:12 ` Eric Biggers 2019-12-19 0:12 ` [f2fs-dev] " Eric Biggers 2019-12-19 0:31 ` Satya Tangirala 2019-12-19 0:31 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-22 0:16 ` Eric Biggers 2019-12-22 0:16 ` [f2fs-dev] " Eric Biggers 2020-01-08 14:05 ` [PATCH v6 0/9] Inline Encryption Support Christoph Hellwig 2020-01-08 14:05 ` [f2fs-dev] " Christoph Hellwig 2020-01-08 18:43 ` Satya Tangirala 2020-01-08 18:43 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-01-17 8:52 ` Christoph Hellwig 2020-01-17 8:52 ` [f2fs-dev] " Christoph Hellwig 2020-02-01 0:53 ` Satya Tangirala 2020-02-01 0:53 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-03 9:15 ` Christoph Hellwig 2020-02-03 9:15 ` [f2fs-dev] " Christoph Hellwig 2020-02-04 3:39 ` Satya Tangirala 2020-02-04 3:39 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-04 14:58 ` Christoph Hellwig 2020-02-04 14:58 ` [f2fs-dev] " Christoph Hellwig 2020-02-04 21:21 ` Eric Biggers 2020-02-04 21:21 ` [f2fs-dev] " Eric Biggers 2020-02-05 7:36 ` Eric Biggers 2020-02-05 7:36 ` [f2fs-dev] " Eric Biggers 2020-02-05 18:05 ` Christoph Hellwig 2020-02-05 18:05 ` [f2fs-dev] " Christoph Hellwig 2020-02-21 12:30 ` Satya Tangirala 2020-02-21 12:30 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-21 14:20 ` Christoph Hellwig 2020-02-21 14:20 ` [f2fs-dev] " Christoph Hellwig
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200117083221.GA324@infradead.org \ --to=hch@infradead.org \ --cc=bmuthuku@qti.qualcomm.com \ --cc=boojin.kim@samsung.com \ --cc=darrick.wong@oracle.com \ --cc=ebiggers@kernel.org \ --cc=kuohong.wang@mediatek.com \ --cc=linux-block@vger.kernel.org \ --cc=linux-f2fs-devel@lists.sourceforge.net \ --cc=linux-fscrypt@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-scsi@vger.kernel.org \ --cc=martin.petersen@oracle.com \ --cc=satyat@google.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.