All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Yann E. MO RIN" <yann.morin.1998@free.fr>
To: Oleg Lyovin <ovlevin@salutedevices.com>
Cc: olegartys@gmail.com, John Keeping <john@metanate.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH 1/1] package/fscryptctl: add choice between v1 and v2 versions
Date: Sun, 29 Oct 2023 00:50:38 +0200	[thread overview]
Message-ID: <20231028225038.GG687180@scaer> (raw)
In-Reply-To: <20231027161010.980901-1-ovlevin@salutedevices.com>

Oleg, All,

+John for commit b832f6eb9d

On 2023-10-27 19:10 +0300, Oleg Lyovin via buildroot spake thusly:
> b832f6eb9d bumped fscryptctl to the version that
> does not support fscrypt v1. However, there may be
> active users that already has v1 configuration without
> an ability to migrate to v2, so the above change would
> break them.
> 
> This patch adds the option to choice which fscryptctl
> version to use assuming v2 by default.

It is very exceptional that we allow chosing a version, so you'll need
to provide more explanations than "without an ability to migrate to v2".

Why is it not possible for those users to migrate?

Note that commit b832f6eb9d stated:
    There are unlikely to be many v1 users and the format has some known
    vulnerabilities so no compatibility option is included.

So this was a mindful decision not to provide such compatibility,
especially because of the vulnerability issues.

In the odd case that we do indeed need to provide that choice, then
there is no need to split in two, as it is totally possible to write
everything in a single .mk in conditional blocs. E.g. (elided for
brevity):

    choice
        prompt "fscryptctl variant"

    config BR2_PACKAGE_FSCRYPTCTL_V1
        bool "v1 (deprecated)"

    config BR2_PACKAGE_FSCRYPTCTL_V2
        bool "v2"

    endchoice

    config BR2_PACKAGE_FSCRYPTCTL_VERSION
        string
        default "f037dcf4354ce8f25d0f371b58dfe7a7ac27576f" if BR2_PACKAGE_FSCRYPTCTL_V1
        default "1.0.0" if BR2_PACKAGE_FSCRYPTCTL_V2

and then in the .mk:

    FSCRYPTCTL_VERSION = $(call qstrip,$(BR2_PACKAGE_FSCRYPTCTL_VERSION))
    FSCRYPTCTL_GIT_VERSION = $(if $(BR2_PACKAGE_FSCRYPTCTL_V2),v)$(FSCRYPTCTL_VERSION)
    FSCRYPTCTL_SITE = $(call github,google,fscryptctl,$(FSCRYPTCTL_GIT_VERSION))

    FSCRYPTCTL_CFLAGS = $(TARGET_CFLAGS)
    ifeq ($(BR2_PACKAGE_FSCRYPTCTL_V2),y)
    FSCRYPTCTL_CFLAGS += -std=c99
    endif

    define FSCRYPTCTL_BUILD_CMDS
        $(MAKE) -C $(@D) $(TARGET_CONFIGURE_OPTS) \
            CFLAGS="$(FSCRYPTCTL_CFLAGS)" fscryptctl
    endef

    define FSCRYPTCTL_INSTALL_TARGET_CMDS
        $(INSTALL) -m 0755 -D $(@D)/fscryptctl $(TARGET_DIR)/usr/bin/fscryptctl
    endef

    $(eval $(generic-package))

Toally untested, and missing all the rest of the usual variables...

But of course, this still needs a good explanations why this is needed.

Regards,
Yann E. MORIN.

> Signed-off-by: Oleg Lyovin <ovlevin@salutedevices.com>
> ---
>  package/fscryptctl/Config.in                  | 22 +++++++++++++++++++
>  package/fscryptctl/fscryptctl.mk              | 22 +------------------
>  .../fscryptctl_v1/fscryptctl_v1.hash          |  3 +++
>  .../fscryptctl/fscryptctl_v1/fscryptctl_v1.mk | 20 +++++++++++++++++
>  .../fscryptctl_v2.hash}                       |  2 +-
>  .../fscryptctl/fscryptctl_v2/fscryptctl_v2.mk | 21 ++++++++++++++++++
>  6 files changed, 68 insertions(+), 22 deletions(-)
>  create mode 100644 package/fscryptctl/fscryptctl_v1/fscryptctl_v1.hash
>  create mode 100644 package/fscryptctl/fscryptctl_v1/fscryptctl_v1.mk
>  rename package/fscryptctl/{fscryptctl.hash => fscryptctl_v2/fscryptctl_v2.hash} (81%)
>  create mode 100644 package/fscryptctl/fscryptctl_v2/fscryptctl_v2.mk
> 
> diff --git a/package/fscryptctl/Config.in b/package/fscryptctl/Config.in
> index 91b9ba8ebe..2fb7176ad9 100644
> --- a/package/fscryptctl/Config.in
> +++ b/package/fscryptctl/Config.in
> @@ -10,3 +10,25 @@ config BR2_PACKAGE_FSCRYPTCTL
>  	  (BR2_TARGET_ROOTFS_EXT2_MKFS_OPTIONS="-O encrypt -b 4096")
>  
>  	  https://github.com/google/fscryptctl
> +
> +if BR2_PACKAGE_FSCRYPTCTL
> +
> +choice
> +	prompt "fscryptctl variant"
> +	default BR2_PACKAGE_FSCRYPTCTL_V2
> +	help
> +	  Select the version of fscrypt.
> +
> +config BR2_PACKAGE_FSCRYPTCTL_V2
> +	bool "fscrpyctl_v2"
> +	help
> +	  Support the latest fscrypt v2 implementation.
> +
> +config BR2_PACKAGE_FSCRYPTCTL_V1
> +	bool "fscryptctl_v1"
> +	help
> +	  v1 is deprecated, enable this only for legacy configurations.
> +
> +endchoice
> +
> +endif
> diff --git a/package/fscryptctl/fscryptctl.mk b/package/fscryptctl/fscryptctl.mk
> index 0546f67a1f..e3571f4e07 100644
> --- a/package/fscryptctl/fscryptctl.mk
> +++ b/package/fscryptctl/fscryptctl.mk
> @@ -1,21 +1 @@
> -################################################################################
> -#
> -# fscryptctl
> -#
> -################################################################################
> -
> -FSCRYPTCTL_VERSION = 1.0.0
> -FSCRYPTCTL_SITE = $(call github,google,fscryptctl,v$(FSCRYPTCTL_VERSION))
> -FSCRYPTCTL_LICENSE = Apache-2.0
> -FSCRYPTCTL_LICENSE_FILES = LICENSE
> -
> -define FSCRYPTCTL_BUILD_CMDS
> -	$(MAKE) -C $(@D) $(TARGET_CONFIGURE_OPTS) \
> -		CFLAGS="$(TARGET_CFLAGS) -std=c99" fscryptctl
> -endef
> -
> -define FSCRYPTCTL_INSTALL_TARGET_CMDS
> -	$(INSTALL) -m 0755 -D $(@D)/fscryptctl $(TARGET_DIR)/usr/bin/fscryptctl
> -endef
> -
> -$(eval $(generic-package))
> +include $(sort $(wildcard package/fscryptctl/*/*.mk))
> diff --git a/package/fscryptctl/fscryptctl_v1/fscryptctl_v1.hash b/package/fscryptctl/fscryptctl_v1/fscryptctl_v1.hash
> new file mode 100644
> index 0000000000..f810aaacb5
> --- /dev/null
> +++ b/package/fscryptctl/fscryptctl_v1/fscryptctl_v1.hash
> @@ -0,0 +1,3 @@
> +# Locally calculated
> +sha256  970a8febbcbf315313711d8a7ee3d954dea593d8087744d3cba65f6cb4bebcc1  fscryptctl_v1-f037dcf4354ce8f25d0f371b58dfe7a7ac27576f.tar.gz
> +sha256  cfc7749b96f63bd31c3c42b5c471bf756814053e847c10f3eb003417bc523d30  LICENSE
> diff --git a/package/fscryptctl/fscryptctl_v1/fscryptctl_v1.mk b/package/fscryptctl/fscryptctl_v1/fscryptctl_v1.mk
> new file mode 100644
> index 0000000000..c1c1ab34f2
> --- /dev/null
> +++ b/package/fscryptctl/fscryptctl_v1/fscryptctl_v1.mk
> @@ -0,0 +1,20 @@
> +################################################################################
> +#
> +# fscryptctl_v1
> +#
> +################################################################################
> +
> +FSCRYPTCTL_V1_VERSION = f037dcf4354ce8f25d0f371b58dfe7a7ac27576f
> +FSCRYPTCTL_V1_SITE = $(call github,google,fscryptctl,$(FSCRYPTCTL_V1_VERSION))
> +FSCRYPTCTL_V1_LICENSE = Apache-2.0
> +FSCRYPTCTL_V1_LICENSE_FILES = LICENSE
> +
> +define FSCRYPTCTL_V1_BUILD_CMDS
> +	$(MAKE) -C $(@D) $(TARGET_CONFIGURE_OPTS) fscryptctl
> +endef
> +
> +define FSCRYPTCTL_V1_INSTALL_TARGET_CMDS
> +	$(INSTALL) -m 0755 -D $(@D)/fscryptctl $(TARGET_DIR)/usr/bin/fscryptctl
> +endef
> +
> +$(eval $(generic-package))
> diff --git a/package/fscryptctl/fscryptctl.hash b/package/fscryptctl/fscryptctl_v2/fscryptctl_v2.hash
> similarity index 81%
> rename from package/fscryptctl/fscryptctl.hash
> rename to package/fscryptctl/fscryptctl_v2/fscryptctl_v2.hash
> index 0dcca6893e..61ff022310 100644
> --- a/package/fscryptctl/fscryptctl.hash
> +++ b/package/fscryptctl/fscryptctl_v2/fscryptctl_v2.hash
> @@ -1,3 +1,3 @@
>  # Locally calculated
> -sha256  3828d5ad9b93664b9fec0174fc5d8e96d7b021a7896da74efe18fabe5f01d638  fscryptctl-1.0.0.tar.gz
> +sha256  3828d5ad9b93664b9fec0174fc5d8e96d7b021a7896da74efe18fabe5f01d638  fscryptctl_v2-1.0.0.tar.gz
>  sha256  cfc7749b96f63bd31c3c42b5c471bf756814053e847c10f3eb003417bc523d30  LICENSE
> diff --git a/package/fscryptctl/fscryptctl_v2/fscryptctl_v2.mk b/package/fscryptctl/fscryptctl_v2/fscryptctl_v2.mk
> new file mode 100644
> index 0000000000..847a7bbcda
> --- /dev/null
> +++ b/package/fscryptctl/fscryptctl_v2/fscryptctl_v2.mk
> @@ -0,0 +1,21 @@
> +################################################################################
> +#
> +# fscryptctl_v2
> +#
> +################################################################################
> +
> +FSCRYPTCTL_V2_VERSION = 1.0.0
> +FSCRYPTCTL_V2_SITE = $(call github,google,fscryptctl,v$(FSCRYPTCTL_V2_VERSION))
> +FSCRYPTCTL_V2_LICENSE = Apache-2.0
> +FSCRYPTCTL_V2_LICENSE_FILES = LICENSE
> +
> +define FSCRYPTCTL_V2_BUILD_CMDS
> +	$(MAKE) -C $(@D) $(TARGET_CONFIGURE_OPTS) \
> +		CFLAGS="$(TARGET_CFLAGS) -std=c99" fscryptctl
> +endef
> +
> +define FSCRYPTCTL_V2_INSTALL_TARGET_CMDS
> +	$(INSTALL) -m 0755 -D $(@D)/fscryptctl $(TARGET_DIR)/usr/bin/fscryptctl
> +endef
> +
> +$(eval $(generic-package))
> -- 
> 2.42.0.270.gbcb6cae296
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

       reply	other threads:[~2023-10-28 22:50 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20231027161010.980901-1-ovlevin@salutedevices.com>
2023-10-28 22:50 ` Yann E. MO RIN [this message]
2023-10-29 13:27   ` [Buildroot] [PATCH 1/1] package/fscryptctl: add choice between v1 and v2 versions Oleg Lyovin via buildroot
2023-11-06 13:47     ` Oleg Lyovin via buildroot

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=20231028225038.GG687180@scaer \
    --to=yann.morin.1998@free.fr \
    --cc=buildroot@buildroot.org \
    --cc=john@metanate.com \
    --cc=olegartys@gmail.com \
    --cc=ovlevin@salutedevices.com \
    --cc=thomas.petazzoni@bootlin.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: link
Be 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.