[dm-crypt] [cryptsetup PATCH] Make BitLocker support optional

[dm-crypt] [cryptsetup PATCH] Make BitLocker support optional

[Phil Sutter]

The mandatory dependency on libiconv introduced by it makes it feasible
to support optional compilation.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 configure.ac      | 12 ++++++++++++
 lib/Makemodule.am |  6 +++++-
 lib/bitlk/bitlk.h | 39 +++++++++++++++++++++++++++++++++++++++
 lib/setup.c       |  6 ++++++
 src/cryptsetup.c  | 14 ++++++++++++--
 5 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index 0805bd20d88b8..8e677be810768 100644
--- a/configure.ac
+++ b/configure.ac
@@ -140,6 +140,18 @@ if test "x$enable_ssh_token" = "xyes" -a "x$enable_external_tokens" = "xno"; the
 	AC_MSG_ERROR([Requested LUKS2 ssh-token build, but external tokens are disabled.])
 fi
 
+dnl ==========================================================================
+dnl BitLocker support
+
+AC_ARG_ENABLE([bitlk],
+	AS_HELP_STRING([--disable-bitlk], [disable BitLocker support]),
+	[], [enable_bitlk=yes])
+AM_CONDITIONAL(BITLK, test "x$enable_bitlk" = "xyes")
+
+if test "x$enable_bitlk" = "xyes"; then
+	AC_DEFINE(ENABLE_BITLK, 1, [Build BitLocker support])
+fi
+
 dnl ==========================================================================
 
 AM_GNU_GETTEXT([external],[need-ngettext])
diff --git a/lib/Makemodule.am b/lib/Makemodule.am
index 5b12eae84b594..ed25cce3fd2fd 100644
--- a/lib/Makemodule.am
+++ b/lib/Makemodule.am
@@ -107,5 +107,9 @@ libcryptsetup_la_SOURCES = \
 	lib/luks2/luks2.h		\
 	lib/utils_blkid.c		\
 	lib/utils_blkid.h		\
-	lib/bitlk/bitlk.h		\
+	lib/bitlk/bitlk.h
+
+if BITLK
+libcryptsetup_la_SOURCES += \
 	lib/bitlk/bitlk.c
+endif
diff --git a/lib/bitlk/bitlk.h b/lib/bitlk/bitlk.h
index 57ba92e3833bf..518f97fe1a2b4 100644
--- a/lib/bitlk/bitlk.h
+++ b/lib/bitlk/bitlk.h
@@ -114,6 +114,8 @@ struct bitlk_metadata {
 	struct bitlk_fvek *fvek;
 };
 
+#ifdef ENABLE_BITLK
+
 int BITLK_read_sb(struct crypt_device *cd, struct bitlk_metadata *params);
 
 int BITLK_dump(struct crypt_device *cd, struct device *device, struct bitlk_metadata *params);
@@ -142,4 +144,41 @@ void BITLK_bitlk_fvek_free(struct bitlk_fvek *fvek);
 void BITLK_bitlk_vmk_free(struct bitlk_vmk *vmk);
 void BITLK_bitlk_metadata_free(struct bitlk_metadata *params);
 
+#else
+
+static inline int
+BITLK_read_sb(struct crypt_device *cd, struct bitlk_metadata *params) { return -ENOTSUP; }
+
+static inline int
+BITLK_dump(struct crypt_device *cd, struct device *device, struct bitlk_metadata *params) { return -ENOTSUP; }
+
+static inline int
+BITLK_get_volume_key(struct crypt_device *cd,
+		     const char *password,
+		     size_t passwordLen,
+		     const struct bitlk_metadata *params,
+		     struct volume_key **open_fvek_key) { return -ENOTSUP; }
+
+static inline int
+BITLK_activate_by_passphrase(struct crypt_device *cd,
+			     const char *name,
+			     const char *password,
+			     size_t passwordLen,
+			     const struct bitlk_metadata *params,
+			     uint32_t flags) { return -ENOTSUP; }
+
+static inline int
+BITLK_activate_by_volume_key(struct crypt_device *cd,
+			     const char *name,
+			     const char *volume_key,
+			     size_t volume_key_size,
+			     const struct bitlk_metadata *params,
+			     uint32_t flags) { return -ENOTSUP; }
+
+static inline void BITLK_bitlk_fvek_free(struct bitlk_fvek *fvek) {}
+static inline void BITLK_bitlk_vmk_free(struct bitlk_vmk *vmk) {}
+static inline void BITLK_bitlk_metadata_free(struct bitlk_metadata *params) {}
+
+#endif
+
 #endif
diff --git a/lib/setup.c b/lib/setup.c
index a5dfd843743a0..997cecf158026 100644
--- a/lib/setup.c
+++ b/lib/setup.c
@@ -320,7 +320,11 @@ static int isINTEGRITY(const char *type)
 
 static int isBITLK(const char *type)
 {
+#ifdef ENABLE_BITLK
 	return (type && !strcmp(CRYPT_BITLK, type));
+#else
+	return 0;
+#endif
 }
 
 static int _onlyLUKS(struct crypt_device *cd, uint32_t cdflags)
@@ -1470,8 +1474,10 @@ int crypt_init_by_name_and_header(struct crypt_device **cd,
 			(*cd)->type = strdup(CRYPT_TCRYPT);
 		else if (!strncmp(CRYPT_INTEGRITY, dmd.uuid, sizeof(CRYPT_INTEGRITY)-1))
 			(*cd)->type = strdup(CRYPT_INTEGRITY);
+#ifdef ENABLE_BITLK
 		else if (!strncmp(CRYPT_BITLK, dmd.uuid, sizeof(CRYPT_BITLK)-1))
 			(*cd)->type = strdup(CRYPT_BITLK);
+#endif
 		else
 			log_dbg(NULL, "Unknown UUID set, some parameters are not set.");
 	} else
diff --git a/src/cryptsetup.c b/src/cryptsetup.c
index e785dc3be2fd1..d4d2ddaf665ac 100644
--- a/src/cryptsetup.c
+++ b/src/cryptsetup.c
@@ -517,6 +517,7 @@ out:
 	return r;
 }
 
+#ifdef ENABLE_BITLK
 static int action_open_bitlk(void)
 {
 	struct crypt_device *cd = NULL;
@@ -576,6 +577,7 @@ out:
 	crypt_free(cd);
 	return r;
 }
+#endif
 
 static int tcryptDump_with_volume_key(struct crypt_device *cd)
 {
@@ -649,6 +651,7 @@ out:
 	return r;
 }
 
+#ifdef ENABLE_BITLK
 static int bitlkDump_with_volume_key(struct crypt_device *cd)
 {
 	char *vk = NULL, *password = NULL;
@@ -733,6 +736,7 @@ out:
 	crypt_free(cd);
 	return r;
 }
+#endif
 
 static int action_close(void)
 {
@@ -2443,10 +2447,12 @@ static int action_open(void)
 		if (action_argc < 2 && !ARG_SET(OPT_TEST_PASSPHRASE_ID))
 			goto out;
 		return action_open_tcrypt();
+#ifdef ENABLE_BITLK
 	} else if (!strcmp(device_type, "bitlk")) {
 		if (action_argc < 2 && !ARG_SET(OPT_TEST_PASSPHRASE_ID))
 			goto out;
 		return action_open_bitlk();
+#endif
 	} else
 		r = -ENOENT;
 out:
@@ -3515,7 +3521,9 @@ static struct action_type {
 	{ ISLUKS_ACTION,	action_isLuks,		1, 0, N_("<device>"), N_("tests <device> for LUKS partition header") },
 	{ LUKSDUMP_ACTION,	action_luksDump,	1, 1, N_("<device>"), N_("dump LUKS partition information") },
 	{ TCRYPTDUMP_ACTION,	action_tcryptDump,	1, 1, N_("<device>"), N_("dump TCRYPT device information") },
+#ifdef ENABLE_BITLK
 	{ BITLKDUMP_ACTION,	action_bitlkDump,	1, 1, N_("<device>"), N_("dump BITLK device information") },
+#endif
 	{ SUSPEND_ACTION,	action_luksSuspend,	1, 1, N_("<device>"), N_("Suspend LUKS device and wipe key (all IOs are frozen)") },
 	{ RESUME_ACTION,	action_luksResume,	1, 1, N_("<device>"), N_("Resume suspended LUKS device") },
 	{ HEADERBACKUP_ACTION,	action_luksBackup,	1, 1, N_("<device>"), N_("Backup LUKS device header and keyslots") },
@@ -3812,13 +3820,15 @@ int main(int argc, const char **argv)
 	} else if (!strcmp(aname, "tcryptOpen")) {
 		aname = OPEN_ACTION;
 		device_type = "tcrypt";
+	} else if (!strcmp(aname, "tcryptDump")) {
+		device_type = "tcrypt";
+#ifdef ENABLE_BITLK
 	} else if (!strcmp(aname, "bitlkOpen")) {
 		aname = OPEN_ACTION;
 		device_type = "bitlk";
-	} else if (!strcmp(aname, "tcryptDump")) {
-		device_type = "tcrypt";
 	} else if (!strcmp(aname, "bitlkDump")) {
 		device_type = "bitlk";
+#endif
 	} else if (!strcmp(aname, "remove") ||
 		   !strcmp(aname, "plainClose") ||
 		   !strcmp(aname, "luksClose") ||
-- 
2.33.1

_______________________________________________
dm-crypt mailing list -- dm-crypt@saout.de
To unsubscribe send an email to dm-crypt-leave@saout.de

[dm-crypt] Re: [cryptsetup PATCH] Make BitLocker support optional

[Milan Broz]

Hi,

Support for all formats is mandatory (the pain to support various kernel configuration is already enough),
so sorry, but I will not accept this patch.

Also you cannot disable commands on CLI this way, it breaks user interface.
(Command can fail, but must not dissappear.)

What issues this solves have here? Why you cannot link it?

We use only some specific functions so the solution can be just to implement this internally.

Milan



On 28/10/2021 01:29, Phil Sutter wrote:
> The mandatory dependency on libiconv introduced by it makes it feasible
> to support optional compilation.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>   configure.ac      | 12 ++++++++++++
>   lib/Makemodule.am |  6 +++++-
>   lib/bitlk/bitlk.h | 39 +++++++++++++++++++++++++++++++++++++++
>   lib/setup.c       |  6 ++++++
>   src/cryptsetup.c  | 14 ++++++++++++--
>   5 files changed, 74 insertions(+), 3 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 0805bd20d88b8..8e677be810768 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -140,6 +140,18 @@ if test "x$enable_ssh_token" = "xyes" -a "x$enable_external_tokens" = "xno"; the
>   	AC_MSG_ERROR([Requested LUKS2 ssh-token build, but external tokens are disabled.])
>   fi
>   
> +dnl ==========================================================================
> +dnl BitLocker support
> +
> +AC_ARG_ENABLE([bitlk],
> +	AS_HELP_STRING([--disable-bitlk], [disable BitLocker support]),
> +	[], [enable_bitlk=yes])
> +AM_CONDITIONAL(BITLK, test "x$enable_bitlk" = "xyes")
> +
> +if test "x$enable_bitlk" = "xyes"; then
> +	AC_DEFINE(ENABLE_BITLK, 1, [Build BitLocker support])
> +fi
> +
>   dnl ==========================================================================
>   
>   AM_GNU_GETTEXT([external],[need-ngettext])
> diff --git a/lib/Makemodule.am b/lib/Makemodule.am
> index 5b12eae84b594..ed25cce3fd2fd 100644
> --- a/lib/Makemodule.am
> +++ b/lib/Makemodule.am
> @@ -107,5 +107,9 @@ libcryptsetup_la_SOURCES = \
>   	lib/luks2/luks2.h		\
>   	lib/utils_blkid.c		\
>   	lib/utils_blkid.h		\
> -	lib/bitlk/bitlk.h		\
> +	lib/bitlk/bitlk.h
> +
> +if BITLK
> +libcryptsetup_la_SOURCES += \
>   	lib/bitlk/bitlk.c
> +endif
> diff --git a/lib/bitlk/bitlk.h b/lib/bitlk/bitlk.h
> index 57ba92e3833bf..518f97fe1a2b4 100644
> --- a/lib/bitlk/bitlk.h
> +++ b/lib/bitlk/bitlk.h
> @@ -114,6 +114,8 @@ struct bitlk_metadata {
>   	struct bitlk_fvek *fvek;
>   };
>   
> +#ifdef ENABLE_BITLK
> +
>   int BITLK_read_sb(struct crypt_device *cd, struct bitlk_metadata *params);
>   
>   int BITLK_dump(struct crypt_device *cd, struct device *device, struct bitlk_metadata *params);
> @@ -142,4 +144,41 @@ void BITLK_bitlk_fvek_free(struct bitlk_fvek *fvek);
>   void BITLK_bitlk_vmk_free(struct bitlk_vmk *vmk);
>   void BITLK_bitlk_metadata_free(struct bitlk_metadata *params);
>   
> +#else
> +
> +static inline int
> +BITLK_read_sb(struct crypt_device *cd, struct bitlk_metadata *params) { return -ENOTSUP; }
> +
> +static inline int
> +BITLK_dump(struct crypt_device *cd, struct device *device, struct bitlk_metadata *params) { return -ENOTSUP; }
> +
> +static inline int
> +BITLK_get_volume_key(struct crypt_device *cd,
> +		     const char *password,
> +		     size_t passwordLen,
> +		     const struct bitlk_metadata *params,
> +		     struct volume_key **open_fvek_key) { return -ENOTSUP; }
> +
> +static inline int
> +BITLK_activate_by_passphrase(struct crypt_device *cd,
> +			     const char *name,
> +			     const char *password,
> +			     size_t passwordLen,
> +			     const struct bitlk_metadata *params,
> +			     uint32_t flags) { return -ENOTSUP; }
> +
> +static inline int
> +BITLK_activate_by_volume_key(struct crypt_device *cd,
> +			     const char *name,
> +			     const char *volume_key,
> +			     size_t volume_key_size,
> +			     const struct bitlk_metadata *params,
> +			     uint32_t flags) { return -ENOTSUP; }
> +
> +static inline void BITLK_bitlk_fvek_free(struct bitlk_fvek *fvek) {}
> +static inline void BITLK_bitlk_vmk_free(struct bitlk_vmk *vmk) {}
> +static inline void BITLK_bitlk_metadata_free(struct bitlk_metadata *params) {}
> +
> +#endif
> +
>   #endif
> diff --git a/lib/setup.c b/lib/setup.c
> index a5dfd843743a0..997cecf158026 100644
> --- a/lib/setup.c
> +++ b/lib/setup.c
> @@ -320,7 +320,11 @@ static int isINTEGRITY(const char *type)
>   
>   static int isBITLK(const char *type)
>   {
> +#ifdef ENABLE_BITLK
>   	return (type && !strcmp(CRYPT_BITLK, type));
> +#else
> +	return 0;
> +#endif
>   }
>   
>   static int _onlyLUKS(struct crypt_device *cd, uint32_t cdflags)
> @@ -1470,8 +1474,10 @@ int crypt_init_by_name_and_header(struct crypt_device **cd,
>   			(*cd)->type = strdup(CRYPT_TCRYPT);
>   		else if (!strncmp(CRYPT_INTEGRITY, dmd.uuid, sizeof(CRYPT_INTEGRITY)-1))
>   			(*cd)->type = strdup(CRYPT_INTEGRITY);
> +#ifdef ENABLE_BITLK
>   		else if (!strncmp(CRYPT_BITLK, dmd.uuid, sizeof(CRYPT_BITLK)-1))
>   			(*cd)->type = strdup(CRYPT_BITLK);
> +#endif
>   		else
>   			log_dbg(NULL, "Unknown UUID set, some parameters are not set.");
>   	} else
> diff --git a/src/cryptsetup.c b/src/cryptsetup.c
> index e785dc3be2fd1..d4d2ddaf665ac 100644
> --- a/src/cryptsetup.c
> +++ b/src/cryptsetup.c
> @@ -517,6 +517,7 @@ out:
>   	return r;
>   }
>   
> +#ifdef ENABLE_BITLK
>   static int action_open_bitlk(void)
>   {
>   	struct crypt_device *cd = NULL;
> @@ -576,6 +577,7 @@ out:
>   	crypt_free(cd);
>   	return r;
>   }
> +#endif
>   
>   static int tcryptDump_with_volume_key(struct crypt_device *cd)
>   {
> @@ -649,6 +651,7 @@ out:
>   	return r;
>   }
>   
> +#ifdef ENABLE_BITLK
>   static int bitlkDump_with_volume_key(struct crypt_device *cd)
>   {
>   	char *vk = NULL, *password = NULL;
> @@ -733,6 +736,7 @@ out:
>   	crypt_free(cd);
>   	return r;
>   }
> +#endif
>   
>   static int action_close(void)
>   {
> @@ -2443,10 +2447,12 @@ static int action_open(void)
>   		if (action_argc < 2 && !ARG_SET(OPT_TEST_PASSPHRASE_ID))
>   			goto out;
>   		return action_open_tcrypt();
> +#ifdef ENABLE_BITLK
>   	} else if (!strcmp(device_type, "bitlk")) {
>   		if (action_argc < 2 && !ARG_SET(OPT_TEST_PASSPHRASE_ID))
>   			goto out;
>   		return action_open_bitlk();
> +#endif
>   	} else
>   		r = -ENOENT;
>   out:
> @@ -3515,7 +3521,9 @@ static struct action_type {
>   	{ ISLUKS_ACTION,	action_isLuks,		1, 0, N_("<device>"), N_("tests <device> for LUKS partition header") },
>   	{ LUKSDUMP_ACTION,	action_luksDump,	1, 1, N_("<device>"), N_("dump LUKS partition information") },
>   	{ TCRYPTDUMP_ACTION,	action_tcryptDump,	1, 1, N_("<device>"), N_("dump TCRYPT device information") },
> +#ifdef ENABLE_BITLK
>   	{ BITLKDUMP_ACTION,	action_bitlkDump,	1, 1, N_("<device>"), N_("dump BITLK device information") },
> +#endif
>   	{ SUSPEND_ACTION,	action_luksSuspend,	1, 1, N_("<device>"), N_("Suspend LUKS device and wipe key (all IOs are frozen)") },
>   	{ RESUME_ACTION,	action_luksResume,	1, 1, N_("<device>"), N_("Resume suspended LUKS device") },
>   	{ HEADERBACKUP_ACTION,	action_luksBackup,	1, 1, N_("<device>"), N_("Backup LUKS device header and keyslots") },
> @@ -3812,13 +3820,15 @@ int main(int argc, const char **argv)
>   	} else if (!strcmp(aname, "tcryptOpen")) {
>   		aname = OPEN_ACTION;
>   		device_type = "tcrypt";
> +	} else if (!strcmp(aname, "tcryptDump")) {
> +		device_type = "tcrypt";
> +#ifdef ENABLE_BITLK
>   	} else if (!strcmp(aname, "bitlkOpen")) {
>   		aname = OPEN_ACTION;
>   		device_type = "bitlk";
> -	} else if (!strcmp(aname, "tcryptDump")) {
> -		device_type = "tcrypt";
>   	} else if (!strcmp(aname, "bitlkDump")) {
>   		device_type = "bitlk";
> +#endif
>   	} else if (!strcmp(aname, "remove") ||
>   		   !strcmp(aname, "plainClose") ||
>   		   !strcmp(aname, "luksClose") ||
> 
_______________________________________________
dm-crypt mailing list -- dm-crypt@saout.de
To unsubscribe send an email to dm-crypt-leave@saout.de

[dm-crypt] Re: [cryptsetup PATCH] Make BitLocker support optional

[Phil Sutter]

Hi Milan,

On Thu, Oct 28, 2021 at 09:14:10AM +0200, Milan Broz wrote:
> Support for all formats is mandatory (the pain to support various kernel configuration is already enough),
> so sorry, but I will not accept this patch.

I can relate but in this case the default is enabled so unless someone
really cares nothing changes.

> Also you cannot disable commands on CLI this way, it breaks user interface.
> (Command can fail, but must not dissappear.)

Sounds doable, assuming the above is not a final NACK. ;)

> What issues this solves have here? Why you cannot link it?

On an embedded device with uClibc I need libiconv which is 1.4MB in
size. I was hoping to avoid having to ship this rather large library.
While it's awesome that cryptsetup now supports bitlk partitions, I
don't think it will see much use on embedded devices (e.g. a small file
server).

> We use only some specific functions so the solution can be just to implement this internally.

Converting passphrases to utf16 is mandatory for bitlk support, right?

In general, I'm not sure if all this is feasible - libcryptsetup is
already 1.9MB and maintaining a mini-iconv is error-prone and likely to
remain mostly untested.

Cheers, Phil
_______________________________________________
dm-crypt mailing list -- dm-crypt@saout.de
To unsubscribe send an email to dm-crypt-leave@saout.de

[dm-crypt] Re: [cryptsetup PATCH] Make BitLocker support optional

[Milan Broz]

On 29/10/2021 23:31, Phil Sutter wrote:
> Hi Milan,
> 
> On Thu, Oct 28, 2021 at 09:14:10AM +0200, Milan Broz wrote:
>> Support for all formats is mandatory (the pain to support various kernel configuration is already enough),
>> so sorry, but I will not accept this patch.
> 
> I can relate but in this case the default is enabled so unless someone
> really cares nothing changes.

All formats in libcryptsetup are intentionally always available.
This was my intention since the beginning I started to add external
format support (loopaes, truecrypt etc).

...

>> What issues this solves have here? Why you cannot link it?
> 
> On an embedded device with uClibc I need libiconv which is 1.4MB in
> size. I was hoping to avoid having to ship this rather large library.
> While it's awesome that cryptsetup now supports bitlk partitions, I
> don't think it will see much use on embedded devices (e.g. a small file
> server).

So the whole problem is just to save 1.4M? I thought you cannot compile it at all.

Then this is not really something what I think is really important
- cryptsetup is not indented to be used in super-small embedded devices.
(But yes, we try to avoid big libraries dependences. But bitlk support is
mandatory function now.)

You can always add own patches obviously, it is OSS, but this is not going
to be merged upstream.

>> We use only some specific functions so the solution can be just to implement this internally.
> 
> Converting passphrases to utf16 is mandatory for bitlk support, right?

Not only passphrases, labels etc are stored in utf16. But it is only small
subset of iconv we need.
  
> In general, I'm not sure if all this is feasible - libcryptsetup is
> already 1.9MB and maintaining a mini-iconv is error-prone and likely to
> remain mostly untested.

Systemd implements own utf functions (not sure why).
I would better add similar to libcryptsetup just for bitlk format (with unit test),
but not sure it is worth to spend time here... (IOW remove iconv dependence completely.)

(Anyway, cc to Vojta, who wrote this code.)

Milan
_______________________________________________
dm-crypt mailing list -- dm-crypt@saout.de
To unsubscribe send an email to dm-crypt-leave@saout.de