Re: [PATCH] added 'isar-cfg-userpw' package

[Jan Kiszka <jan.kiszka@siemens.com>]

On 25.02.19 09:44, Claudius Heine wrote:
> Hi Jan,
> 
> Quoting Jan Kiszka (2019-02-25 09:07:35)
>> On 23.02.19 11:42, Jan Kiszka wrote:
>>> On 18.02.19 17:21, [ext] claudius.heine.ext@siemens.com wrote:
>>>> From: Claudius Heine <ch@denx.de>
>>>>
>>>> With this package setting of arbitrary user passwords should be
>>>> possible.
>>>>
>>>> To do this use the 'CFG_USER_PW' variable as described in the user
>>>> manual.
>>>>
>>>> Signed-off-by: Claudius Heine <ch@denx.de>
>>>> ---
>>>>    doc/user_manual.md                            |  1 +
>>>>    meta-isar/conf/local.conf.sample              |  2 ++
>>>>    meta/classes/isar-image.bbclass               |  2 +-
>>>>    .../isar-cfg-userpw/files/postinst.tmpl       | 15 ++++++++++++
>>>>    .../isar-cfg-userpw/isar-cfg-userpw.bb        | 23 +++++++++++++++++++
>>>>    5 files changed, 42 insertions(+), 1 deletion(-)
>>>>    create mode 100644 meta/recipes-support/isar-cfg-userpw/files/postinst.tmpl
>>>>    create mode 100644 meta/recipes-support/isar-cfg-userpw/isar-cfg-userpw.bb
>>>>
>>>> diff --git a/doc/user_manual.md b/doc/user_manual.md
>>>> index db0bf85..53bb36a 100644
>>>> --- a/doc/user_manual.md
>>>> +++ b/doc/user_manual.md
>>>> @@ -328,6 +328,7 @@ Some other variables include:
>>>>     - `DISTRO_APT_PREMIRRORS` - The preferred mirror (append it to the default
>>>> URI in the format `ftp.debian.org my.preferred.mirror`. This variable is
>>>> optional.
>>>>     - `CFG_ROOT_PW` - The encrypted root password to be set. To encrypt
>>>> password use `mkpasswd`. You find `mkpasswd` in the `whois` package of Debian.
>>>> If the variable is empty, root login is passwordless.
>>>>     - `CFG_ROOT_LOCKED` - If set to `1` the root account will be locked.
>>>> + - `CFG_USER_PW` - A space separated list of user names and encrypted
>>>> passwords separated by a colon. (e.g. `username1:encryptedpw1
>>>> username2:encryptedpw2`)
>>>>
>>>>    ---
>>>>
>>>> diff --git a/meta-isar/conf/local.conf.sample b/meta-isar/conf/local.conf.sample
>>>> index e5827aa..494a283 100644
>>>> --- a/meta-isar/conf/local.conf.sample
>>>> +++ b/meta-isar/conf/local.conf.sample
>>>> @@ -178,3 +178,5 @@ ISAR_CROSS_COMPILE ?= "0"
>>>>    #   mkpasswd -m sha512crypt -R 10000
>>>>    # mkpasswd is part of the 'whois' package of Debian
>>>>    CFG_ROOT_PW ?=
>>>> "$6$rounds=10000$RXeWrnFmkY$DtuS/OmsAS2cCEDo0BF5qQsizIrq6jPgXnwv3PHqREJeKd1sXdHX/ayQtuQWVDHe0KIO0/sVH8dvQm1KthF0d/"
>>>>
>>>> +# Set user 'isar' password to 'isar':
>>>> +CFG_USER_PW ?=
>>>> "isar:$6$rounds=10000$WMnSt8s9nLE$M/0eQVs0f05VpW8uzscs54GUwzhh/gjN3Vb85QEIIh1XihyvE.Xw4reJSxHqWcP0I0CnllKhseg6SRcGIIx7P1"
>>>>
>>>> diff --git a/meta/classes/isar-image.bbclass b/meta/classes/isar-image.bbclass
>>>> index cdd1651..0100d0b 100644
>>>> --- a/meta/classes/isar-image.bbclass
>>>> +++ b/meta/classes/isar-image.bbclass
>>>> @@ -17,7 +17,7 @@ SRC_URI += "${@ cfg_script(d) }"
>>>>
>>>>    DEPENDS += "${IMAGE_INSTALL} ${IMAGE_TRANSIENT_PACKAGES}"
>>>>
>>>> -IMAGE_TRANSIENT_PACKAGES += "isar-cfg-localepurge isar-cfg-rootpw"
>>>> +IMAGE_TRANSIENT_PACKAGES += "isar-cfg-localepurge isar-cfg-rootpw
>>>> isar-cfg-userpw"
>>>>
>>>>    WORKDIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}"
>>>>
>>>> diff --git a/meta/recipes-support/isar-cfg-userpw/files/postinst.tmpl
>>>> b/meta/recipes-support/isar-cfg-userpw/files/postinst.tmpl
>>>> new file mode 100644
>>>> index 0000000..47fffd0
>>>> --- /dev/null
>>>> +++ b/meta/recipes-support/isar-cfg-userpw/files/postinst.tmpl
>>>> @@ -0,0 +1,15 @@
>>>> +#!/bin/sh
>>>> +set -e
>>>> +
>>>> +USER_ENTRIES='${CFG_USER_PW} '
>>>> +
>>>> +while true; do
>>>> +    USER_ENTRY="${USER_ENTRIES%% *}" # First element of list
>>>> +    USER_ENTRIES="${USER_ENTRIES#${USER_ENTRY} }" # Rest of list
>>>> +
>>>> +    if [ -z "${USER_ENTRY}" ]; then
>>>> +        break
>>>> +    fi
>>>> +
>>>> +    printf '%s' "${USER_ENTRY}" | chpasswd -e
>>>> +done
>>>> diff --git a/meta/recipes-support/isar-cfg-userpw/isar-cfg-userpw.bb
>>>> b/meta/recipes-support/isar-cfg-userpw/isar-cfg-userpw.bb
>>>> new file mode 100644
>>>> index 0000000..75b0446
>>>> --- /dev/null
>>>> +++ b/meta/recipes-support/isar-cfg-userpw/isar-cfg-userpw.bb
>>>> @@ -0,0 +1,23 @@
>>>> +# This software is a part of ISAR.
>>>> +
>>>> +DESCRIPTION = "Isar configuration package for user passwords"
>>>> +MAINTAINER = "isar-users <isar-users@googlegroups.com>"
>>>> +DEBIAN_DEPENDS = "passwd"
>>>> +
>>>> +SRC_URI = "file://postinst.tmpl"
>>>> +
>>>> +TEMPLATE_FILES = "postinst.tmpl"
>>>> +TEMPLATE_VARS = "CFG_USER_PW"
>>>> +
>>>> +CFG_USER_PW ?= ""
>>>> +
>>>> +python() {
>>>> +    # Enforce CFG_USER_PW to be a single space separated array
>>>> +    d.setVar("CFG_USER_PW", " ".join(d.getVar("CFG_USER_PW", True).split()))
>>>> +}
>>>> +
>>>> +inherit dpkg-raw
>>>> +
>>>> +do_install() {
>>>> +    echo "intentionally left blank"
>>>> +}
>>>>
>>>
>>> Missed this until I had to deal with it: This does not allow per-image password
>>> configuration because there is only one, hard-coded isar-cfg-userpw package that
>>> all images pull. E.g., how to build a release (root account locked) and a debug
>>> image (well-known insecure or empty password) at the same time now?
>>>
>>> We rather need to change the logic to pass the control variables from the host
>>> down into the chroot during installation where the transient package can then
>>> evaluate them. Or model this - as a special case - without a package.
>>>
>>> Before the release, we should at least prove if the current recipe interface can
>>> be maintained with the above requirement, so that we do not break it again right
>>> after that.
>>>
>>
>> The same conceptual issue applies to isar-cfg-localepurge: LOCALE_GEN and
>> LOCALE_DEFAULT should be configurable on a per-image basis, not a per-build.
> 
> You are right! I haven't considered that.
> 
> Normally you would not have a 'debug' image and a 'release' image, but
> different multi/local configurations for that. Having debug images and
> release images is a anti-pattern for bb based projects IMO and should
> not be done in Isar.

This is not true. In the end, you will always have two images of that kind, 
often defined by different package sets, set in the respective image recipes.

> 
> But of course if you now have a '*-debug' and '*-release' multiconfig,
> you cannot build that in parallel if one package is build with two
> different variables.
> 
> And that exactly hits the mark with the problem I have with the way Isar
> uses multiconfigs and tries to share packages from different
> multiconfigs.
> 
> IMO if you want to continue doing it that way, you would need to have a
> 'isar-cfg-localpurge-debug' and a 'isar-cfg-userpw-debug'. And do that
> for all possible other configurations you want to build in parallel...

Awkward. We need to stop this weird patterns which require too much boilerplate 
recipes to achieve very simple things. Let's just make these variables per-image.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux