From: Wesley Miaw <[email protected]>
Add data device start block index to dm-verity target parameters to support
verity targets where the data does not begin at sector 0 of the block device.
Also fix the hash block index computation so it takes into account data offsets.
Signed-off-by: Wesley Miaw <[email protected]>
---
Documentation/device-mapper/verity.txt | 8 ++++-
drivers/md/dm-verity.c | 32 +++++++++++++++--------
2 files changed, 27 insertions(+), 13 deletions(-)
--- a/drivers/md/dm-verity.c 2012-08-07 16:03:03.778759000 -0700
+++ b/drivers/md/dm-verity.c 2012-08-07 17:32:02.130176956 -0700
@@ -491,7 +491,7 @@ static int verity_map(struct dm_target *
io->bio = bio;
io->orig_bi_end_io = bio->bi_end_io;
io->orig_bi_private = bio->bi_private;
- io->block = bio->bi_sector >> (v->data_dev_block_bits - SECTOR_SHIFT);
+ io->block = (bio->bi_sector - v->data_start) >> (v->data_dev_block_bits - SECTOR_SHIFT);
io->n_blocks = bio->bi_size >> v->data_dev_block_bits;
bio->bi_end_io = verity_end_io;
@@ -641,6 +641,7 @@ static void verity_dtr(struct dm_target
* <hash device>
* <data block size>
* <hash block size>
+ * <data start block>
* <the number of data blocks>
* <hash start block>
* <algorithm>
@@ -671,8 +672,8 @@ static int verity_ctr(struct dm_target *
goto bad;
}
- if (argc != 10) {
- ti->error = "Invalid argument count: exactly 10 arguments required";
+ if (argc != 11) {
+ ti->error = "Invalid argument count: exactly 11 arguments required";
r = -EINVAL;
goto bad;
}
@@ -718,6 +719,15 @@ static int verity_ctr(struct dm_target *
v->hash_dev_block_bits = ffs(num) - 1;
if (sscanf(argv[5], "%llu%c", &num_ll, &dummy) != 1 ||
+ num_ll << (v->data_dev_block_bits - SECTOR_SHIFT) !=
+ (sector_t)num_ll << (v->data_dev_block_bits - SECTOR_SHIFT)) {
+ ti->error = "Invalid data start";
+ r = -EINVAL;
+ goto bad;
+ }
+ v->data_start = num_ll << (v->data_dev_block_bits - SECTOR_SHIFT);
+
+ if (sscanf(argv[6], "%llu%c", &num_ll, &dummy) != 1 ||
num_ll << (v->data_dev_block_bits - SECTOR_SHIFT) !=
(sector_t)num_ll << (v->data_dev_block_bits - SECTOR_SHIFT)) {
ti->error = "Invalid data blocks";
@@ -732,7 +742,7 @@ static int verity_ctr(struct dm_target *
goto bad;
}
- if (sscanf(argv[6], "%llu%c", &num_ll, &dummy) != 1 ||
+ if (sscanf(argv[7], "%llu%c", &num_ll, &dummy) != 1 ||
num_ll << (v->hash_dev_block_bits - SECTOR_SHIFT) !=
(sector_t)num_ll << (v->hash_dev_block_bits - SECTOR_SHIFT)) {
ti->error = "Invalid hash start";
@@ -741,7 +751,7 @@ static int verity_ctr(struct dm_target *
}
v->hash_start = num_ll;
- v->alg_name = kstrdup(argv[7], GFP_KERNEL);
+ v->alg_name = kstrdup(argv[8], GFP_KERNEL);
if (!v->alg_name) {
ti->error = "Cannot allocate algorithm name";
r = -ENOMEM;
@@ -770,23 +780,23 @@ static int verity_ctr(struct dm_target *
r = -ENOMEM;
goto bad;
}
- if (strlen(argv[8]) != v->digest_size * 2 ||
- hex2bin(v->root_digest, argv[8], v->digest_size)) {
+ if (strlen(argv[9]) != v->digest_size * 2 ||
+ hex2bin(v->root_digest, argv[9], v->digest_size)) {
ti->error = "Invalid root digest";
r = -EINVAL;
goto bad;
}
- if (strcmp(argv[9], "-")) {
- v->salt_size = strlen(argv[9]) / 2;
+ if (strcmp(argv[10], "-")) {
+ v->salt_size = strlen(argv[10]) / 2;
v->salt = kmalloc(v->salt_size, GFP_KERNEL);
if (!v->salt) {
ti->error = "Cannot allocate salt";
r = -ENOMEM;
goto bad;
}
- if (strlen(argv[9]) != v->salt_size * 2 ||
- hex2bin(v->salt, argv[9], v->salt_size)) {
+ if (strlen(argv[10]) != v->salt_size * 2 ||
+ hex2bin(v->salt, argv[10], v->salt_size)) {
ti->error = "Invalid salt";
r = -EINVAL;
goto bad;
--- a/Documentation/device-mapper/verity.txt 2012-08-08 11:02:48.558883756 -0700
+++ b/Documentation/device-mapper/verity.txt 2012-08-08 11:13:01.259982498 -0700
@@ -9,7 +9,7 @@ Construction Parameters
=======================
<version> <dev> <hash_dev>
<data_block_size> <hash_block_size>
- <num_data_blocks> <hash_start_block>
+ <data_start_block> <num_data_blocks> <hash_start_block>
<algorithm> <digest> <salt>
<version>
@@ -41,6 +41,10 @@ Construction Parameters
<hash_block_size>
The size of a hash block in bytes.
+<data_start_block>
+ This is the offset, in <data_block_size>-blocks, from the start of data_dev
+ to the first block of the data.
+
<num_data_blocks>
The number of data blocks on the data device. Additional blocks are
inaccessible. You can place hashes to the same partition as data, in this
@@ -136,7 +140,7 @@ Example
=======
Set up a device:
# dmsetup create vroot --readonly --table \
- "0 2097152 verity 1 /dev/sda1 /dev/sda2 4096 4096 262144 1 sha256 "\
+ "0 2097152 verity 1 /dev/sda1 /dev/sda2 4096 4096 0 262144 1 sha256 "\
"4392712ba01368efdf14b05c76f9e4df0d53664630b5d48632ed17a137f39076 "\
"1234000000000000000000000000000000000000000000000000000000000000"
Hi
The problem with the patch is that it changes interface to the userspace
tool. The userspace tool veritysetup already exists in recent cryptsetup
package, so we can't change the interface - you should change the patch so
that the starting data block is the last argument and the argument is
optional - so that it is compatible with the existing userspace too.
Another thing --- do we need this patch at all? You can create a dm-linear
device and stack existing dm-verity on the top of it to get the same
effect of changing data starting block. Is there some reason why you can't
use dm-linear device and why you need this patch?
Mikulas
On Wed, 8 Aug 2012, Wesley Miaw wrote:
> From: Wesley Miaw <[email protected]>
>
> Add data device start block index to dm-verity target parameters to support
> verity targets where the data does not begin at sector 0 of the block device.
> Also fix the hash block index computation so it takes into account data offsets.
>
> Signed-off-by: Wesley Miaw <[email protected]>
> ---
> Documentation/device-mapper/verity.txt | 8 ++++-
> drivers/md/dm-verity.c | 32 +++++++++++++++--------
> 2 files changed, 27 insertions(+), 13 deletions(-)
> --- a/drivers/md/dm-verity.c 2012-08-07 16:03:03.778759000 -0700
> +++ b/drivers/md/dm-verity.c 2012-08-07 17:32:02.130176956 -0700
> @@ -491,7 +491,7 @@ static int verity_map(struct dm_target *
> io->bio = bio;
> io->orig_bi_end_io = bio->bi_end_io;
> io->orig_bi_private = bio->bi_private;
> - io->block = bio->bi_sector >> (v->data_dev_block_bits - SECTOR_SHIFT);
> + io->block = (bio->bi_sector - v->data_start) >> (v->data_dev_block_bits - SECTOR_SHIFT);
> io->n_blocks = bio->bi_size >> v->data_dev_block_bits;
>
> bio->bi_end_io = verity_end_io;
> @@ -641,6 +641,7 @@ static void verity_dtr(struct dm_target
> * <hash device>
> * <data block size>
> * <hash block size>
> + * <data start block>
> * <the number of data blocks>
> * <hash start block>
> * <algorithm>
> @@ -671,8 +672,8 @@ static int verity_ctr(struct dm_target *
> goto bad;
> }
>
> - if (argc != 10) {
> - ti->error = "Invalid argument count: exactly 10 arguments required";
> + if (argc != 11) {
> + ti->error = "Invalid argument count: exactly 11 arguments required";
> r = -EINVAL;
> goto bad;
> }
> @@ -718,6 +719,15 @@ static int verity_ctr(struct dm_target *
> v->hash_dev_block_bits = ffs(num) - 1;
>
> if (sscanf(argv[5], "%llu%c", &num_ll, &dummy) != 1 ||
> + num_ll << (v->data_dev_block_bits - SECTOR_SHIFT) !=
> + (sector_t)num_ll << (v->data_dev_block_bits - SECTOR_SHIFT)) {
> + ti->error = "Invalid data start";
> + r = -EINVAL;
> + goto bad;
> + }
> + v->data_start = num_ll << (v->data_dev_block_bits - SECTOR_SHIFT);
> +
> + if (sscanf(argv[6], "%llu%c", &num_ll, &dummy) != 1 ||
> num_ll << (v->data_dev_block_bits - SECTOR_SHIFT) !=
> (sector_t)num_ll << (v->data_dev_block_bits - SECTOR_SHIFT)) {
> ti->error = "Invalid data blocks";
> @@ -732,7 +742,7 @@ static int verity_ctr(struct dm_target *
> goto bad;
> }
>
> - if (sscanf(argv[6], "%llu%c", &num_ll, &dummy) != 1 ||
> + if (sscanf(argv[7], "%llu%c", &num_ll, &dummy) != 1 ||
> num_ll << (v->hash_dev_block_bits - SECTOR_SHIFT) !=
> (sector_t)num_ll << (v->hash_dev_block_bits - SECTOR_SHIFT)) {
> ti->error = "Invalid hash start";
> @@ -741,7 +751,7 @@ static int verity_ctr(struct dm_target *
> }
> v->hash_start = num_ll;
>
> - v->alg_name = kstrdup(argv[7], GFP_KERNEL);
> + v->alg_name = kstrdup(argv[8], GFP_KERNEL);
> if (!v->alg_name) {
> ti->error = "Cannot allocate algorithm name";
> r = -ENOMEM;
> @@ -770,23 +780,23 @@ static int verity_ctr(struct dm_target *
> r = -ENOMEM;
> goto bad;
> }
> - if (strlen(argv[8]) != v->digest_size * 2 ||
> - hex2bin(v->root_digest, argv[8], v->digest_size)) {
> + if (strlen(argv[9]) != v->digest_size * 2 ||
> + hex2bin(v->root_digest, argv[9], v->digest_size)) {
> ti->error = "Invalid root digest";
> r = -EINVAL;
> goto bad;
> }
>
> - if (strcmp(argv[9], "-")) {
> - v->salt_size = strlen(argv[9]) / 2;
> + if (strcmp(argv[10], "-")) {
> + v->salt_size = strlen(argv[10]) / 2;
> v->salt = kmalloc(v->salt_size, GFP_KERNEL);
> if (!v->salt) {
> ti->error = "Cannot allocate salt";
> r = -ENOMEM;
> goto bad;
> }
> - if (strlen(argv[9]) != v->salt_size * 2 ||
> - hex2bin(v->salt, argv[9], v->salt_size)) {
> + if (strlen(argv[10]) != v->salt_size * 2 ||
> + hex2bin(v->salt, argv[10], v->salt_size)) {
> ti->error = "Invalid salt";
> r = -EINVAL;
> goto bad;
> --- a/Documentation/device-mapper/verity.txt 2012-08-08 11:02:48.558883756 -0700
> +++ b/Documentation/device-mapper/verity.txt 2012-08-08 11:13:01.259982498 -0700
> @@ -9,7 +9,7 @@ Construction Parameters
> =======================
> <version> <dev> <hash_dev>
> <data_block_size> <hash_block_size>
> - <num_data_blocks> <hash_start_block>
> + <data_start_block> <num_data_blocks> <hash_start_block>
> <algorithm> <digest> <salt>
>
> <version>
> @@ -41,6 +41,10 @@ Construction Parameters
> <hash_block_size>
> The size of a hash block in bytes.
>
> +<data_start_block>
> + This is the offset, in <data_block_size>-blocks, from the start of data_dev
> + to the first block of the data.
> +
> <num_data_blocks>
> The number of data blocks on the data device. Additional blocks are
> inaccessible. You can place hashes to the same partition as data, in this
> @@ -136,7 +140,7 @@ Example
> =======
> Set up a device:
> # dmsetup create vroot --readonly --table \
> - "0 2097152 verity 1 /dev/sda1 /dev/sda2 4096 4096 262144 1 sha256 "\
> + "0 2097152 verity 1 /dev/sda1 /dev/sda2 4096 4096 0 262144 1 sha256 "\
> "4392712ba01368efdf14b05c76f9e4df0d53664630b5d48632ed17a137f39076 "\
> "1234000000000000000000000000000000000000000000000000000000000000"
>
>
>
On 08/08/2012 08:46 PM, Mikulas Patocka wrote:
> The problem with the patch is that it changes interface to the userspace
> tool. The userspace tool veritysetup already exists in recent cryptsetup
> package, so we can't change the interface - you should change the patch so
> that the starting data block is the last argument and the argument is
> optional - so that it is compatible with the existing userspace too.
yes. Please never change interface without at least increasing target version.
I have to add userspace support as well to veritysetup and we need a way
how to detect that option is supported by running kernel.
Milan
On Aug 8, 2012, at 1:31 PM, Milan Broz wrote:
> On 08/08/2012 08:46 PM, Mikulas Patocka wrote:
>
>> The problem with the patch is that it changes interface to the userspace
>> tool. The userspace tool veritysetup already exists in recent cryptsetup
>> package, so we can't change the interface - you should change the patch so
>> that the starting data block is the last argument and the argument is
>> optional - so that it is compatible with the existing userspace too.
>
> yes. Please never change interface without at least increasing target version.
>
> I have to add userspace support as well to veritysetup and we need a way
> how to detect that option is supported by running kernel.
Understood. Thank you for the feedback. I will attempt a new patch version which addresses these issues. I also found that I did not correct the last-block boundary check so I will re-submit my patch with that as well.
I did modify veritysetup on my own so the format and verify commands will work with regular files on disk instead of having to mount through loop devices.
--
Wesley Miaw
On 08/08/2012 10:46 PM, Wesley Miaw wrote:
> On Aug 8, 2012, at 1:31 PM, Milan Broz wrote:
> I did modify veritysetup on my own so the format and verify commands will work with regular files on disk instead of having to mount through loop devices.
Which veritysetup? In upstream (cryptsetup repository) it allocates loop automatically.
(And for userspace verification it doesn't need loop at all.)
Anyway, please send a patch for userspace as well then ;-)
Milan
On Aug 8, 2012, at 1:56 PM, Milan Broz wrote:
> On 08/08/2012 10:46 PM, Wesley Miaw wrote:
>
>> I did modify veritysetup on my own so the format and verify commands will work with regular files on disk instead of having to mount through loop devices.
>
> Which veritysetup? In upstream (cryptsetup repository) it allocates loop automatically.
> (And for userspace verification it doesn't need loop at all.)
>
> Anyway, please send a patch for userspace as well then ;-)
I grabbed cryptsetup from http://code.google.com/p/cryptsetup as what I read said that is the most recent. And then modified the code in there because the final block device images need to combine the file system, hash data, and some metadata into a single image and I don't want my users to need root privileges.
I can send a separate patch of those changes, but I'm not sure to where? Also to the LKML?
Thanks,
--
Wesley Miaw
On Aug 8, 2012, at 1:31 PM, Milan Broz wrote:
> On 08/08/2012 08:46 PM, Mikulas Patocka wrote:
>
>> The problem with the patch is that it changes interface to the userspace
>> tool. The userspace tool veritysetup already exists in recent cryptsetup
>> package, so we can't change the interface - you should change the patch so
>> that the starting data block is the last argument and the argument is
>> optional - so that it is compatible with the existing userspace too.
>
> yes. Please never change interface without at least increasing target version.
>
> I have to add userspace support as well to veritysetup and we need a way
> how to detect that option is supported by running kernel.
Apologies if the version increment is incorrect; I was not sure if the minor or patch number should be incremented. I assume the different version number is what would be used to detect if the data offset option is supported. Thanks.
From: Wesley Miaw <[email protected]>
Add data device start block index as optional dm-verity target parameters to
support verity targets where the data does not begin at sector 0 of the block
device.
Also fix the hash block index computations so they take into account any data
offset.
Signed-off-by: Wesley Miaw <[email protected]>
---
Documentation/device-mapper/verity.txt | 8 ++++++-
drivers/md/dm-verity.c | 24 ++++++++++++++++++-----
2 files changed, 26 insertions(+), 6 deletions(-)
--- a/drivers/md/dm-verity.c 2012-08-07 16:03:03.778759000 -0700
+++ b/drivers/md/dm-verity.c 2012-08-08 17:04:16.344682266 -0700
@@ -477,7 +477,7 @@ static int verity_map(struct dm_target *
return -EIO;
}
- if ((bio->bi_sector + bio_sectors(bio)) >>
+ if ((bio->bi_sector - v->data_start + bio_sectors(bio)) >>
(v->data_dev_block_bits - SECTOR_SHIFT) > v->data_blocks) {
DMERR_LIMIT("io out of range");
return -EIO;
@@ -491,7 +491,7 @@ static int verity_map(struct dm_target *
io->bio = bio;
io->orig_bi_end_io = bio->bi_end_io;
io->orig_bi_private = bio->bi_private;
- io->block = bio->bi_sector >> (v->data_dev_block_bits - SECTOR_SHIFT);
+ io->block = (bio->bi_sector - v->data_start) >> (v->data_dev_block_bits - SECTOR_SHIFT);
io->n_blocks = bio->bi_size >> v->data_dev_block_bits;
bio->bi_end_io = verity_end_io;
@@ -646,6 +646,7 @@ static void verity_dtr(struct dm_target
* <algorithm>
* <digest>
* <salt> Hex string or "-" if no salt.
+ * <data start block> Optional. The default is zero.
*/
static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv)
{
@@ -671,8 +672,8 @@ static int verity_ctr(struct dm_target *
goto bad;
}
- if (argc != 10) {
- ti->error = "Invalid argument count: exactly 10 arguments required";
+ if (argc != 10 && argc != 11) {
+ ti->error = "Invalid argument count: 10 or 11 arguments required";
r = -EINVAL;
goto bad;
}
@@ -793,6 +794,19 @@ static int verity_ctr(struct dm_target *
}
}
+ if (argc == 11) {
+ if (sscanf(argv[10], "%llu%c", &num_ll, &dummy) != 1 ||
+ num_ll << (v->data_dev_block_bits - SECTOR_SHIFT) !=
+ (sector_t)num_ll << (v->data_dev_block_bits - SECTOR_SHIFT)) {
+ ti->error = "Invalid data start";
+ r = -EINVAL;
+ goto bad;
+ }
+ v->data_start = num_ll << (v->data_dev_block_bits - SECTOR_SHIFT);
+ } else {
+ v->data_start = 0;
+ }
+
v->hash_per_block_bits =
fls((1 << v->hash_dev_block_bits) / v->digest_size) - 1;
@@ -875,7 +889,7 @@ bad:
static struct target_type verity_target = {
.name = "verity",
- .version = {1, 0, 0},
+ .version = {1, 1, 0},
.module = THIS_MODULE,
.ctr = verity_ctr,
.dtr = verity_dtr,
--- a/Documentation/device-mapper/verity.txt 2012-08-08 11:02:48.558883756 -0700
+++ b/Documentation/device-mapper/verity.txt 2012-08-08 16:50:04.114864090 -0700
@@ -11,6 +11,7 @@ Construction Parameters
<data_block_size> <hash_block_size>
<num_data_blocks> <hash_start_block>
<algorithm> <digest> <salt>
+ [<data_start_block>]
<version>
This is the type of the on-disk hash format.
@@ -62,6 +63,10 @@ Construction Parameters
<salt>
The hexadecimal encoding of the salt value.
+<data_start_block>
+ This is the offset, in <data_block_size>-blocks, from the start of data_dev
+ to the first block of the data.
+
Theory of operation
===================
@@ -138,7 +143,8 @@ Set up a device:
# dmsetup create vroot --readonly --table \
"0 2097152 verity 1 /dev/sda1 /dev/sda2 4096 4096 262144 1 sha256 "\
"4392712ba01368efdf14b05c76f9e4df0d53664630b5d48632ed17a137f39076 "\
- "1234000000000000000000000000000000000000000000000000000000000000"
+ "1234000000000000000000000000000000000000000000000000000000000000 "\
+ "0"
A command line tool veritysetup is available to compute or verify
the hash tree or activate the kernel device. This is available from
On Aug 8, 2012, at 1:56 PM, Milan Broz wrote:
> On 08/08/2012 10:46 PM, Wesley Miaw wrote:
>
>> I did modify veritysetup on my own so the format and verify commands will work with regular files on disk instead of having to mount through loop devices.
>
> Which veritysetup? In upstream (cryptsetup repository) it allocates loop automatically.
> (And for userspace verification it doesn't need loop at all.)
>
> Anyway, please send a patch for userspace as well then ;-)
This isn't as polished because I pretty much just added support to do what I needed. I'm not sure if the LKML is the right place to post, so let me know if I should send this somewhere else.
Your previous email implied that veritysetup would need a way to determine if the data offset option is supported; I did not modify veritysetup to support this idea as I didn't need it.
Thanks.
From: Wesley Miaw <[email protected]>
Allow veritysetup format and verify commands to directly operate on regular
files instead of requiring mounts through loop devices.
Signed-off-by: Wesley Miaw <[email protected]>
---
cryptsetup/lib/internal.h | 1
cryptsetup/lib/libcryptsetup.h | 22 ++++
cryptsetup/lib/libcryptsetup.sym | 2
cryptsetup/lib/setup.c | 133 ++++++++++++++++++++++++++++-
cryptsetup/lib/utils.c | 12 ++
cryptsetup/src/veritysetup.c | 23 +++--
6 files changed, 183 insertions(+), 10 deletions(-)
--- a/cryptsetup/lib/internal.h 2012-08-08 17:11:20.366392301 -0700
+++ b/cryptsetup/lib/internal.h 2012-08-06 16:17:35.154719491 -0700
@@ -76,6 +76,7 @@ ssize_t read_blockwise(int fd, void *_bu
ssize_t write_lseek_blockwise(int fd, char *buf, size_t count, off_t offset);
int device_ready(struct crypt_device *cd, const char *device, int mode);
int device_size(const char *device, uint64_t *size);
+int file_size(const char *filename, uint64_t *size);
unsigned crypt_getpagesize(void);
--- a/cryptsetup/lib/libcryptsetup.h 2012-08-08 17:11:20.375392929 -0700
+++ b/cryptsetup/lib/libcryptsetup.h 2012-08-06 16:17:35.159720699 -0700
@@ -56,6 +57,19 @@ struct crypt_device; /* crypt device han
int crypt_init(struct crypt_device **cd, const char *device);
/**
+ * Initial crypt device handle from a file and check if provided file exists.
+ *
+ * @param cd Returns pointer to crypt device handle.
+ * @param filename Path to the backing file.
+ *
+ * @return @e 0 on success or negative errno value otherwise.
+ *
+ * @note Note that logging is not initialized here, possible messages uses
+ * default log function.
+ */
+int crypt_initfile(struct crypt_device **cd, const char *filename);
+
+/**
* Initialize crypt device handle from provided active device name,
* and, optionally, from separate metadata (header) device
* and check if provided device exists.
@@ -237,6 +251,15 @@ void crypt_set_password_verify(struct cr
int crypt_set_data_device(struct crypt_device *cd, const char *device);
/**
+ * Set data file
+ * For VERITY it is data file when hash device is separated.
+ *
+ * @param cd crypt device handle
+ * @param filename path to data file
+ */
+int crypt_set_data_file(struct crypt_device *cd, const char *device);
+
+/**
* @defgroup rng "Cryptsetup RNG"
*
* @addtogroup rng
--- a/cryptsetup/lib/libcryptsetup.sym 2012-08-08 17:11:20.375392930 -0700
+++ b/cryptsetup/lib/libcryptsetup.sym 2012-08-06 16:17:35.160720941 -0700
@@ -1,6 +1,7 @@
CRYPTSETUP_1.0 {
global:
crypt_init;
+ crypt_initfile;
crypt_init_by_name;
crypt_init_by_name_and_header;
crypt_set_log_callback;
@@ -13,6 +14,7 @@ CRYPTSETUP_1.0 {
crypt_set_password_verify;
crypt_set_uuid;
crypt_set_data_device;
+ crypt_set_data_file;
crypt_memory_lock;
crypt_format;
--- a/cryptsetup/lib/setup.c 2012-08-08 17:11:20.428396640 -0700
+++ b/cryptsetup/lib/setup.c 2012-08-06 16:17:35.192728669 -0700
@@ -25,6 +25,8 @@
#include <stdarg.h>
#include <fcntl.h>
#include <errno.h>
+#include <sys/types.h>
+#include <sys/stat.h>
#include "libcryptsetup.h"
#include "luks.h"
@@ -585,6 +587,56 @@ bad:
return r;
}
+int crypt_initfile(struct crypt_device **cd, const char *filename)
+{
+ struct crypt_device *h = NULL;
+ int fd;
+ struct stat st;
+ int r;
+
+ if (!cd)
+ return -EINVAL;
+
+ if (stat(filename, &st) < 0) {
+ log_err(NULL, _("File %s doesn't exist or access denied.\n"), filename);
+ return -EINVAL;
+ }
+
+ log_dbg("Trying to open and write file %s.", filename);
+ fd = open(filename, O_RDWR);
+ if (fd < 0) {
+ log_err(NULL, _("Cannot open file %s for writeable access.\n"), filename);
+ return -EINVAL;
+ }
+ close(fd);
+
+ log_dbg("Allocating crypt device %s context.", filename);
+
+ if (!(h = malloc(sizeof(struct crypt_device))))
+ return -ENOMEM;
+
+ memset(h, 0, sizeof(*h));
+ h->loop_device_fd = -1;
+ h->loop_metadata_device_fd = -1;
+ h->device = strdup(filename);
+ if (!h->device) {
+ r = -ENOMEM;
+ goto bad;
+ }
+ h->iteration_time = 1000;
+ h->password_verify = 0;
+ h->tries = 3;
+ h->rng_type = crypt_random_default_key_rng();
+ *cd = h;
+ return 0;
+bad:
+ if (h) {
+ free(h->device);
+ }
+ free(h);
+ return r;
+}
+
static int crypt_check_data_device_size(struct crypt_device *cd)
{
int r;
@@ -606,6 +658,27 @@ static int crypt_check_data_device_size(
return r;
}
+static int crypt_check_data_file_size(struct crypt_device *cd)
+{
+ int r;
+ uint64_t size, size_min;
+
+ /* Check data device size, require at least one sector */
+ size_min = crypt_get_data_offset(cd) << SECTOR_SHIFT ?: SECTOR_SIZE;
+
+ r = file_size(crypt_get_device_name(cd), &size);
+ if (r < 0)
+ return r;
+
+ if (size < size_min) {
+ log_err(cd, _("Header detected but device %s is too small.\n"),
+ crypt_get_device_name(cd));
+ return -EINVAL;
+ }
+
+ return r;
+}
+
int crypt_set_data_device(struct crypt_device *cd, const char *device)
{
char *data_device = NULL;
@@ -641,6 +714,52 @@ int crypt_set_data_device(struct crypt_d
return crypt_check_data_device_size(cd);
}
+int crypt_set_data_file(struct crypt_device *cd, const char *filename)
+{
+ int fd;
+ struct stat st;
+
+ log_dbg("Setting ciphertext data file to %s.", filename ?: "(none");
+
+ if (!isVERITY(cd->type)) {
+ log_err(cd, _("This operation is not supported for this device type.\n"));
+ return -EINVAL;
+ }
+
+ /* metadata device must be set */
+ if (!cd->device || !filename)
+ return -EINVAL;
+
+ if (stat(filename, &st) < 0) {
+ log_err(NULL, _("File %s doesn't exist or access denied.\n"), filename);
+ return -EINVAL;
+ }
+
+ log_dbg("Trying to open and read file %s.", filename);
+ fd = open(filename, O_RDONLY);
+ if (fd < 0) {
+ log_err(NULL, _("Cannot open file %s for read access.\n"), filename);
+ return -EINVAL;
+ }
+ close(fd);
+
+ if (!cd->metadata_device) {
+ cd->metadata_device = cd->device;
+ cd->loop_metadata_device_fd = cd->loop_device_fd;
+ } else {
+ free(cd->device);
+ if (cd->loop_device_fd != -1)
+ close(cd->loop_device_fd);
+ }
+
+ cd->device = strdup(filename);
+ if (!cd->device)
+ return -ENOMEM;
+ cd->loop_device_fd = -1;
+
+ return crypt_check_data_file_size(cd);
+}
+
static int _crypt_load_luks1(struct crypt_device *cd, int require_header, int repair)
{
struct luks_phdr hdr;
@@ -1080,12 +1199,18 @@ static int _crypt_format_verity(struct c
return -ENOMEM;
r = crypt_set_data_device(cd, params->data_device);
- if (r)
- return r;
+ if (r) {
+ r = crypt_set_data_file(cd, params->data_device);
+ if (r)
+ return r;
+ }
if (!params->data_size) {
r = device_size(cd->device, &data_device_size);
- if (r < 0)
- return r;
+ if (r < 0) {
+ r = file_size(cd->device, &data_device_size);
+ if (r < 0)
+ return r;
+ }
cd->verity_hdr.data_size = data_device_size / params->data_block_size;
} else
--- a/cryptsetup/lib/utils.c 2012-08-08 17:11:20.433396990 -0700
+++ b/cryptsetup/lib/utils.c 2012-08-06 16:17:35.202731084 -0700
@@ -316,6 +316,18 @@ int device_size(const char *device, uint
return r;
}
+int file_size(const char *filename, uint64_t *size)
+{
+ struct stat st;
+
+ if (stat(filename, &st) < 0)
+ return -EINVAL;
+
+ *size = (uint64_t)st.st_size;
+
+ return 0;
+}
+
static int get_device_infos(const char *device, enum devcheck device_check,
int *readonly, uint64_t *size)
{
--- a/cryptsetup/src/veritysetup.c 2012-08-08 17:11:20.942432624 -0700
+++ b/cryptsetup/src/veritysetup.c 2012-08-06 16:17:35.235739053 -0700
@@ -142,8 +142,12 @@ static int action_format(int arg)
uint32_t flags = CRYPT_VERITY_CREATE_HASH;
int r;
- if ((r = crypt_init(&cd, action_argv[1])))
- goto out;
+ r = crypt_init(&cd, action_argv[1]);
+ if (r < 0) {
+ r = crypt_initfile(&cd, action_argv[1]);
+ if (r < 0)
+ goto out;
+ }
if (!use_superblock)
flags |= CRYPT_VERITY_NO_HEADER;
@@ -174,8 +178,12 @@ static int _activate(const char *dm_devi
ssize_t hash_size;
int r;
- if ((r = crypt_init(&cd, hash_device)))
- goto out;
+ r = crypt_init(&cd, hash_device);
+ if (r < 0) {
+ r = crypt_initfile(&cd, hash_device);
+ if (r < 0)
+ goto out;
+ }
if (use_superblock) {
params.flags = flags;
@@ -190,8 +198,11 @@ static int _activate(const char *dm_devi
if (r < 0)
goto out;
r = crypt_set_data_device(cd, data_device);
- if (r < 0)
- goto out;
+ if (r < 0) {
+ r = crypt_set_data_file(cd, data_device);
+ if (r < 0)
+ goto out;
+ }
hash_size = crypt_get_volume_key_size(cd);
if (crypt_hex_to_bytes(root_hash, &root_hash_bytes, 0) != hash_size) {
On Thu, Aug 09, 2012 at 12:40:23AM +0000, Wesley Miaw wrote:
> This isn't as polished because I pretty much just added support to do what I
> needed. I'm not sure if the LKML is the right place to post, so let me know if
> I should send this somewhere else.
cryptsetup patches are best sent to the mailing list mentioned at the bottom of
the project page http://code.google.com/p/cryptsetup/ viz. [email protected].
Alasdair
On Thu, Aug 09, 2012 at 12:40:22AM +0000, Wesley Miaw wrote:
> Apologies if the version increment is incorrect; I was not sure if the minor
> or patch number should be incremented. I assume the different version number is
> what would be used to detect if the data offset option is supported. Thanks.
Minor number is the correct thing to increment as you're retaining
compatibility. (Your original version would have needed to increment the major
number to indicate incompatibility with existing userspace code.)
However, first you need to address the second part of Mikulas's email,
namely to make the case for these changes rather than making no kernel
changes and instead stacking the verity target over a linear target.
To put this another way, your patch offers an alternative way to do
something we think the existing kernel can already do, so you need to
advance some reasons why you believe the new alternative method is worth
adding to the kernel and explain this in the patch header.
Alasdair
On 08/09/2012 02:40 AM, Wesley Miaw wrote:
> On Aug 8, 2012, at 1:56 PM, Milan Broz wrote:
>
>> On 08/08/2012 10:46 PM, Wesley Miaw wrote:
>>
>>> I did modify veritysetup on my own so the format and verify
>>> commands will work with regular files on disk instead of having
>>> to mount through loop devices.
>>
>> Which veritysetup? In upstream (cryptsetup repository) it allocates
>> loop automatically. (And for userspace verification it doesn't need
>> loop at all.)
>>
>> Anyway, please send a patch for userspace as well then ;-)
>
> This isn't as polished because I pretty much just added support to do
> what I needed. I'm not sure if the LKML is the right place to post,
> so let me know if I should send this somewhere else.
This is libcryptsetup userspace so better list for this is dmcrypt
mailing list (and/or cc me, I will handle these changes anyway).
The allocated crypto "file" context cannot be later used for some kind
of operations. I do not like this approach musch...
You cannot use file argument for dm-target directly, so your patch
is useful only for your use case but not for anything else.
Anyway, I am sure there is better way how to solve it I just need
to understand what the problem is. What's wrong if code allocates
loop devices (if argument is file)?
Performance? Loop not available? Need root access?
Please explain what's the problem first.
(btw that patch is mangled by a mailer but not a problem now).
> Your previous email implied that veritysetup would need a way to
> determine if the data offset option is supported; I did not modify
> veritysetup to support this idea as I didn't need it.
Once kernel get this option (if you convince upstream:) then I will
the option to userspace. We have to handle many dm-crypt extensions
so not a big problem for dm-verity as well.
Milan
On Aug 8, 2012, at 11:35 PM, Milan Broz wrote:
> On 08/09/2012 02:40 AM, Wesley Miaw wrote:
>>
>>
>> This isn't as polished because I pretty much just added support to do
>> what I needed. I'm not sure if the LKML is the right place to post,
>> so let me know if I should send this somewhere else.
>
> This is libcryptsetup userspace so better list for this is dmcrypt
> mailing list (and/or cc me, I will handle these changes anyway).
I will continue this thread on the dm-crypt mailing list.
Thanks,
--
Wesley Miaw
On Aug 8, 2012, at 7:04 PM, Alasdair G Kergon wrote:
>
> However, first you need to address the second part of Mikulas's email,
> namely to make the case for these changes rather than making no kernel
> changes and instead stacking the verity target over a linear target.
>
> To put this another way, your patch offers an alternative way to do
> something we think the existing kernel can already do, so you need to
> advance some reasons why you believe the new alternative method is worth
> adding to the kernel and explain this in the patch header.
I'm afraid for some reason I didn't get Mikulas's first email, only the reply from Milan which must have been an incomplete quote of Mikulas's text.
For my part, I approached this as porting my existing code to the new dm-verity implementation included in Linux 3.4. The previous code used a data offset as this was convenient and directly supported the block device image format I put together back then.
However I can indeed accomplish what I need using linear, it's just a bit more code. I am not able to measure any runtime performance difference. The primary benefit I can see for is the reduced kernel footprint if the linear target does not need to be included (and my corresponding setup code is about 1/3 smaller). With my cross-compiled kernel the savings is ~1KB; this is obviously a very small benefit.
So I would defer to others on this. While supporting the data offset would be convenient and match well with my specific use case I can live without it and I don't think the size cost is significant enough to matter. I expect to get some feedback from other developers in the coming months regarding my Linux 3.4 integration but I doubt it would change my current opinion on the matter.
Thanks,
--
Wesley Miaw