[dm-crypt] veritysetup support for files

[dm-crypt] veritysetup support for files

[Wesley Miaw]

[-- Attachment #1: Type: text/plain, Size: 10632 bytes --]

Hi,

Included below is an unpolished patch allowing veritysetup format and verify commands to operate on files instead of using loop block devices. This is a continuation of the LKML thread. Milan has already provided feedback around the less than optimal implementation of this patch; but I wanted to get the patch into the mailing list archives and continue the discussion here.

On Aug 8, 2012, at 11:35 PM, Milan Broz wrote:

> 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.

Not requiring root access is one benefit, but not the most important one. The problem I initially ran into is not knowing the exact size to allocate for the hash loop device file. I could compute it, from the data file size and the hash algorithm, but that is inconvenient and error prone. By operating directly on an empty file veritysetup will end up creating a file of the correct size.

Since my target audience is not necessarily proficient in English, Linux, or crypto I have to do my best to make things as simple to execute and follow.

The result I need is one file that contains a filesystem image (this would have already been created ahead of time), a second file that contains the raw hash data for that filesystem image, and a third file containing the dm-verity table parameters. I am creating the last file by redirecting veritysetup stdout to a file.

Thanks,
--
Wesley Miaw

Allow veritysetup format and verify commands to directly operate on regular
files instead of requiring mounts through loop devices.
---
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) {

[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [dm-crypt] veritysetup support for files

[Milan Broz]

On 08/09/2012 07:27 PM, Wesley Miaw wrote:

> The result I need is one file that contains a filesystem image (this
> would have already been created ahead of time), a second file that
> contains the raw hash data for that filesystem image, and a third
> file containing the dm-verity table parameters. I am creating the
> last file by redirecting veritysetup stdout to a file.

Ah, ok. So your only request is that raw hash data image is created
automatically as file (if parameter is not a block device directly)
and that this file doesn't need to be pre-allocated, correct?

I think I can do it even without introducing new library function
(I really do not want next context constructor type here).

I'll reply once I will have something ready for testing.

Milan

Re: [dm-crypt] veritysetup support for files

[Milan Broz]

On 08/10/2012 01:12 PM, Milan Broz wrote:
> On 08/09/2012 07:27 PM, Wesley Miaw wrote:
> 
>> The result I need is one file that contains a filesystem image (this
>> would have already been created ahead of time), a second file that
>> contains the raw hash data for that filesystem image, and a third
>> file containing the dm-verity table parameters. I am creating the
>> last file by redirecting veritysetup stdout to a file.
> 
> Ah, ok. So your only request is that raw hash data image is created
> automatically as file (if parameter is not a block device directly)
> and that this file doesn't need to be pre-allocated, correct?
> 
> I think I can do it even without introducing new library function
> (I really do not want next context constructor type here).
> 
> I'll reply once I will have something ready for testing.

Can you try current git version?

I changed device helpers so loop is now allocated only when it is neccessary
for device-mapper (where block device is always required).

Veritysetup now also create new hash image in format (if not already there).

It still need root privilege (in some command just to initialize DM) but
I think it should works as you need now.

Let me know if it works for you please...

Thanks,
Milan

Re: [dm-crypt] veritysetup support for files

[Wesley Miaw]

[-- Attachment #1: Type: text/plain, Size: 1205 bytes --]

On Aug 13, 2012, at 3:02 AM, Milan Broz wrote:

> On 08/10/2012 01:12 PM, Milan Broz wrote:
>> On 08/09/2012 07:27 PM, Wesley Miaw wrote:
>> 
>>> The result I need is one file that contains a filesystem image (this
>>> would have already been created ahead of time), a second file that
>>> contains the raw hash data for that filesystem image, and a third
>>> file containing the dm-verity table parameters. I am creating the
>>> last file by redirecting veritysetup stdout to a file.
>> 
>> Ah, ok. So your only request is that raw hash data image is created
>> automatically as file (if parameter is not a block device directly)
>> and that this file doesn't need to be pre-allocated, correct?
> 
> Can you try current git version?
> 
> I changed device helpers so loop is now allocated only when it is neccessary
> for device-mapper (where block device is always required).
> 
> Veritysetup now also create new hash image in format (if not already there).
> 
> It still need root privilege (in some command just to initialize DM) but
> I think it should works as you need now.
> 
> Let me know if it works for you please…

Works, great. Thanks, Milan! :)
--
Wesley Miaw


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 495 bytes --]