Fix hook YKFDE_LUKS_OPTIONS, add YKFDE_LUKS_HEADER

[cyrinux]

• Be able to pass several options, long options in YKFDE_LUKS_OPTIONS like "--param1=value1 --allow-discards".
• Add a specifique YKFDE_LUKS_HEADER option that permit to get it working on boot and on resume.

[Vincent43]

I'll prefer to drop YKFDE_LUKS_OPTIONS change (quotes removal) if it's no longer needed for header support. I don't see the need for passing more options there especially if we don't support it while reading cmdline.

[cyrinux]

I'll prefer to drop YKFDE_LUKS_OPTIONS change (quotes removal) if it's no longer needed for header support. I don't see the need for passing more options there especially if we don't support it while reading cmdline.

I'll prefer to drop YKFDE_LUKS_OPTIONS change (quotes removal) if it's no longer needed for header support. I don't see the need for passing more options there especially if we don't support it while reading cmdline.

Hi @Vincent43, thanks for the review.

I would prefer to keep it without quote, this is valid dash syntax that will allow now or i the futur or for specific case the pass several parameters. I mean, YKFDE_LUKS_OPTIONS is exposed in the config file and if we don't permit to pass several parameters finally, its a little weird. Should we remove this from the config file but keep quoted in this case?

[cyrinux]

What do you think of integrating this fresh patch in this hook to handle cmdline? I try it without yubikey with encrypt-sd and works well, https://github.com/maximbaz/pkgbuilds/tree/master/mkinitcpio-encrypt-detached-header https://github.com/maximbaz/pkgbuilds/blob/master/mkinitcpio-encrypt-detached-header/support-detached-header.patch

[Vincent43]

I would prefer to keep it without quote, this is valid dash syntax that will allow now or i the futur or for specific case the pass several parameters

Unquoted variables are against secure shell coding principles which us why I'm trying to avoid them. As we use this option in both bash and ash then safe alternatives like arrays are limited

I mean, YKFDE_LUKS_OPTIONS is exposed in the config file and if we don't permit to pass several parameters finally, its a little weird

It was exposed as alternative for reading cmdline which also may be used in ykfde-open. In practice it always was only about --allow-discards option which is also the only option stock Arch encrypt hook accepts so we're consistent with them. I didn't saw any demand from users to pass anything else there.

Should we remove this from the config file but keep quoted in this case?

Maybe we shouldn't introduce it but if it's already there then I prefer to keep it in case someone is using it. I think I will change description of it to make clear of its purpose.

What do you think of integrating this fresh patch in this hook to handle cmdline?

Yes, this may be better option than exposing YKFDE_LUKS_HEADER in config. I don't have luks volume with detached header so I can't test it but if you can make it work with our hook then it would be nice.

[cyrinux]

I would prefer to keep it without quote, this is valid dash syntax that will allow now or i the futur or for specific case the pass several parameters

Unquoted variables are against secure shell coding principles which us why I'm trying to avoid them. As we use this option in both bash and ash then safe alternatives like arrays are limited

I mean, YKFDE_LUKS_OPTIONS is exposed in the config file and if we don't permit to pass several parameters finally, its a little weird

It was exposed as alternative for reading cmdline which also may be used in ykfde-open. In practice it always was only about --allow-discards option which is also the only option stock Arch encrypt hook accepts so we're consistent with them. I didn't saw any demand from users to pass anything else there.

Should we remove this from the config file but keep quoted in this case?

Maybe we shouldn't introduce it but if it's already there then I prefer to keep it in case someone is using it. I think I will change description of it to make clear of its purpose.

What do you think of integrating this fresh patch in this hook to handle cmdline?

Yes, this may be better option than exposing YKFDE_LUKS_HEADER in config. I don't have luks volume with detached header so I can't test it but if you can make it work with our hook then it would be nice.

Ok :-) I see the point about actual options.
Will try to add support for cmdline in another PR. Would be cool if this one can be merge already 👍🏻

[Vincent43]

I believe reading cmdline is superior solution so if we're going to add it then it's not worth exposing YKFDE_LUKS_HEADER in config right now.

[cyrinux]

Hi guys, will do this asap.