Re: [PATCH 1/4] simplify image-account-extension

[Henning Schild <henning.schild@siemens.com>]

Am Thu, 30 Mar 2023 13:08:01 +0200
schrieb "T. Schaffner" <tobias.schaffner@siemens.com>:

> From: Tobias Schaffner <tobias.schaffner@siemens.com>
> 
> Do the complete user and group creation in python. This allows us to
> drop the encoding and parsing code that was used to make the user and
> group lists available in the shell function.
> 
> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
> ---
>  meta/classes/image-account-extension.bbclass | 368
> +++++++------------ 1 file changed, 124 insertions(+), 244
> deletions(-)
> 
> diff --git a/meta/classes/image-account-extension.bbclass
> b/meta/classes/image-account-extension.bbclass index
> 1a1f704d..d1133bb4 100644 ---
> a/meta/classes/image-account-extension.bbclass +++
> b/meta/classes/image-account-extension.bbclass @@ -1,5 +1,5 @@
>  # This software is a part of ISAR.
> -# Copyright (C) Siemens AG, 2019
> +# Copyright (C) Siemens AG, 2023

2019-2023

did not look at the rest, this feels like it should be tried out ...
functional review, which i would try to do

please make sure that would pass flake8 ... might be hard since it is
"embedded code"

a long time ago we had contributions from Harald Seiler, who managed to
somehow lint python code inside bb code.

Henning

>  #
>  # SPDX-License-Identifier: MIT
>  #
> @@ -25,251 +25,131 @@ GROUPS ??= ""
>  #GROUP_root[gid] = ""
>  #GROUP_root[flags] = "system"
>  
> -def gen_accounts_array(d, listname, entryname, flags,
> verb_flags=None):
> -    from itertools import chain
> -
> -    entries = (d.getVar(listname) or "").split()
> -    return " ".join(
> -        ":".join(
> -            chain(
> -                (entry,),
> -                (
> -                    (",".join(
> -                        (
> -                            d.getVarFlag(entryname + "_" + entry,
> flag, True) or ""
> -                        ).split()
> -                    ) if flag not in (verb_flags or []) else (
> -                        d.getVarFlag(entryname + "_" + entry, flag,
> True) or ""
> -                    )).replace(":","=")
> -                    for flag in flags
> -                ),
> -            )
> -        )
> -        for entry in entries
> -    )
> -
> -# List of space separated entries, where each entry has the format:
> -#
> username:encryptedpassword:expiredate:inactivenumber:userid:groupid:comment:homedir:shell:group1,group2:flag1,flag2
> -IMAGE_ACCOUNTS_USERS =+ "${@gen_accounts_array(d, 'USERS', 'USER',
> ['password',  'expire', 'inactive', 'uid', 'gid', 'comment', 'home',
> 'shell', 'groups', 'flags'], ['password', 'comment', 'home',
> 'shell'])}" - -# List of space separated entries, where each entry
> has the format: -# groupname:groupid:flag1,flag2
> -IMAGE_ACCOUNTS_GROUPS =+ "${@gen_accounts_array(d, 'GROUPS',
> 'GROUP', ['gid', 'flags'])}" - -do_rootfs_install[vardeps] +=
> "${IMAGE_ACCOUNTS_GROUPS} ${IMAGE_ACCOUNTS_USERS}"
> -ROOTFS_POSTPROCESS_COMMAND += "image_postprocess_accounts"
> -image_postprocess_accounts() {
> -    # Create groups
> -    # Add space to the end of the list:
> -    list='${@" ".join(d.getVar('IMAGE_ACCOUNTS_GROUPS').split())} '
> -    while true; do
> -        # Pop first group entry:
> -        list_rest="${list#*:*:* }"
> -        entry="${list%%${list_rest}}"
> -        list="${list_rest}"
> -
> -        if [ -z "${entry}" ]; then
> -            break
> -        fi
> -
> -        # Add colon to the end of the entry and remove trailing
> space:
> -        entry="${entry% }:"
> -
> -        # Decode entries:
> -        name="${entry%%:*}"
> -        entry="${entry#${name}:}"
> -
> -        gid="${entry%%:*}"
> -        entry="${entry#${gid}:}"
> -
> -        flags="${entry%%:*}"
> -        entry="${entry#${flags}:}"
> -
> -        flags=",${flags}," # Needed for searching for substrings
> -
> -        # Check if user already exists:
> -        if grep -q "^${name}:" '${ROOTFSDIR}/etc/group'; then
> -            exists="y"
> -        else
> -            exists="n"
> -        fi
> -
> -        # Create arguments:
> -        set -- # clear arguments
> -
> -        if [ -n "$gid" ]; then
> -            set -- "$@" --gid "$gid"
> -        fi
> -
> -        if [ "n" = "$exists" ]; then
> -            if [ "${flags}" != "${flags%*,system,*}" ]; then
> -                set -- "$@" --system
> -            fi
> -        fi
> -
> -        # Create or modify groups:
> -        if [ "y" = "$exists" ]; then
> -            if [ -z "$@" ]; then
> -                echo "Do not execute groupmod (no changes)."
> -            else
> -                echo "Execute groupmod with \"$@\" for \"$name\""
> -                sudo -E chroot '${ROOTFSDIR}' \
> -                    /usr/sbin/groupmod "$@" "$name"
> -            fi
> -        else
> -            echo "Execute groupadd with \"$@\" for \"$name\""
> -            sudo -E chroot '${ROOTFSDIR}' \
> -                /usr/sbin/groupadd "$@" "$name"
> -        fi
> -    done
> -
> -    # Create users
> -    list='${@" ".join(d.getVar('IMAGE_ACCOUNTS_USERS').split())} '
> -    while true; do
> -        # Pop first user entry:
> -        list_rest="${list#*:*:*:*:*:*:*:*:*:*:* }"
> -        entry="${list%%${list_rest}}"
> -        list="${list_rest}"
> -
> -        if [ -z "${entry}" ]; then
> -            break
> -        fi
> -
> -        # Add colon to the end of the entry and remove trailing
> space:
> -        entry="${entry% }:"
> -
> -        # Decode entries:
> -        name="${entry%%:*}"
> -        entry="${entry#${name}:}"
> -
> -        password="${entry%%:*}"
> -        entry="${entry#${password}:}"
> -
> -        expire="${entry%%:*}"
> -        entry="${entry#${expire}:}"
> -
> -        inactive="${entry%%:*}"
> -        entry="${entry#${inactive}:}"
> -
> -        uid="${entry%%:*}"
> -        entry="${entry#${uid}:}"
> -
> -        gid="${entry%%:*}"
> -        entry="${entry#${gid}:}"
> -
> -        comment="${entry%%:*}"
> -        entry="${entry#${comment}:}"
> -
> -        home="${entry%%:*}"
> -        entry="${entry#${home}:}"
> -
> -        shell="${entry%%:*}"
> -        entry="${entry#${shell}:}"
> -
> -        groups="${entry%%:*}"
> -        entry="${entry#${groups}:}"
> -
> -        flags="${entry%%:*}"
> -        entry="${entry#${flags}:}"
> -
> -        flags=",${flags}," # Needed for searching for substrings
> -
> -        # Check if user already exists:
> -        if grep -q "^${name}:" '${ROOTFSDIR}/etc/passwd'; then
> -            exists="y"
> -        else
> -            exists="n"
> -        fi
> -
> -        # Create arguments:
> -        set -- # clear arguments
> -
> -        if [ -n "$expire" ]; then
> -            set -- "$@" --expiredate "$expire"
> -        fi
> -
> -        if [ -n "$inactive" ]; then
> -            set -- "$@" --inactive "$inactive"
> -        fi
> -
> -        if [ -n "$uid" ]; then
> -            set -- "$@" --uid "$uid"
> -        fi
> -
> -        if [ -n "$gid" ]; then
> -            set -- "$@" --gid "$gid"
> -        fi
> -
> -        if [ -n "$comment" ]; then
> -            set -- "$@" --comment "$comment"
> -        fi
> -
> -        if [ -n "$home" ]; then
> -            if [ "y" = "$exists" ]; then
> -                set -- "$@" --home "$home" --move-home
> -            else
> -                set -- "$@" --home-dir "$home"
> -            fi
> -        fi
> -
> -        if [ -n "$shell" ]; then
> -            set -- "$@" --shell "$shell"
> -        fi
> -
> -        if [ -n "$groups" ]; then
> -            set -- "$@" --groups "$groups"
> -        fi
> -
> -        if [ "n" = "$exists" ]; then
> -            if [ "${flags}" != "${flags%*,system,*}" ]; then
> -                set -- "$@" --system
> -            fi
> -            if [ "${flags}" != "${flags%*,no-create-home,*}" ]; then
> -                set -- "$@" --no-create-home
> -            else
> -                if [ "${flags}" != "${flags%*,create-home,*}" ]; then
> -                    set -- "$@" --create-home
> -                fi
> -            fi
> -        fi
> -
> -        # Create or modify users:
> -        if [ "y" = "$exists" ]; then
> -            if [ -z "$@" ]; then
> -                echo "Do not execute usermod (no changes)."
> -            else
> -                echo "Execute usermod with \"$@\" for \"$name\""
> -                sudo -E chroot '${ROOTFSDIR}' \
> -                    /usr/sbin/usermod "$@" "$name"
> -            fi
> -        else
> -            echo "Execute useradd with \"$@\" for \"$name\""
> -            sudo -E chroot '${ROOTFSDIR}' \
> -                /usr/sbin/useradd "$@" "$name"
> -        fi
> -
> -        # Set password:
> -        if [ -n "$password" -o "${flags}" !=
> "${flags%*,allow-empty-password,*}" ]; then
> -            chpasswd_args="-e"
> -            if [ "${flags}" != "${flags%*,clear-text-password,*}" ];
> then +def image_create_groups(d: "DataSmart") -> None:
> +    """Creates the groups defined in the ``GROUPS`` bitbake variable.
> +
> +    Args:
> +        d (DataSmart): The bitbake datastore.
> +
> +    Returns:
> +        None
> +    """
> +    entries = (d.getVar("GROUPS") or "").split()
> +    rootfsdir = d.getVar("ROOTFSDIR")
> +    chroot = ["sudo", "-E", "chroot", rootfsdir]
> +
> +    for entry in entries:
> +        args = []
> +        group_entry = "GROUP_{}".format(entry)
> +
> +        with open("{}/etc/group".format(rootfsdir), "r") as
> group_file:
> +            exists = any(line.startswith("{}:".format(entry)) for
> line in group_file) +
> +        gid = d.getVarFlag(group_entry, "gid") or ""
> +        if gid:
> +            args.append("--gid")
> +            args.append(gid)
> +
> +        flags = (d.getVarFlag(group_entry, "flags") or "").split()
> +        if "system" in flags:
> +            args.append("--system")
> +
> +        if exists:
> +            if args:
> +                bb.process.run([*chroot, "/usr/sbin/groupmod",
> *args, entry])
> +        else:
> +            bb.process.run([*chroot, "/usr/sbin/groupadd", *args,
> entry]) +
> +
> +def image_create_users(d: "DataSmart") -> None:
> +    """Creates the users defined in the ``USERS`` bitbake variable.
> +
> +    Args:
> +        d (DataSmart): The bitbake datastore.
> +
> +    Returns:
> +        None
> +    """
> +    import hashlib
> +    import crypt
> +
> +    entries = (d.getVar("USERS") or "").split()
> +    rootfsdir = d.getVar("ROOTFSDIR")
> +    chroot = ["sudo", "-E", "chroot", rootfsdir]
> +
> +    for entry in entries:
> +        args = []
> +        user_entry = "USER_{}".format(entry)
> +
> +        with open("{}/etc/passwd".format(rootfsdir), "r") as
> passwd_file:
> +            exists = any(line.startswith("{}:".format(entry)) for
> line in passwd_file) +
> +        def add_user_option(option_name, flag_name):
> +            flag_value = d.getVarFlag(user_entry, flag_name) or ""
> +            if flag_value:
> +                args.append(option_name)
> +                args.append(flag_value)
> +
> +        add_user_option("--expire", "expiredate")
> +        add_user_option("--inactive", "inactive")
> +        add_user_option("--uid", "uid")
> +        add_user_option("--gid", "gid")
> +        add_user_option("--comment", "comment")
> +        add_user_option("--shell", "shell")
> +
> +        groups = d.getVarFlag(user_entry, "groups") or ""
> +        if groups:
> +            args.append("--groups")
> +            args.append(groups.replace(' ', ','))
> +
> +        flags = (d.getVarFlag(user_entry, "flags") or "").split()
> +
> +        if exists:
> +            add_user_option("--home", "home")
> +            if d.getVarFlag(user_entry, "home") or "":
> +                args.append("--move-home")
> +        else:
> +            add_user_option("--home-dir", "home")
> +
> +            if "system" in flags:
> +                args.append("--system")
> +            if "no-create-home" in flags:
> +                args.append("--no-create-home")
> +            if "create-home" in flags:
> +                args.append("--create-home")
> +
> +        if exists:
> +            if args:
> +                bb.process.run([*chroot, "/usr/sbin/usermod", *args,
> entry])
> +        else:
> +            bb.process.run([*chroot, "/usr/sbin/useradd", *args,
> entry]) +
> +        command = [*chroot, "/usr/sbin/chpasswd"]
> +        password = d.getVarFlag(user_entry, "password") or ""
> +        if password or "allow-empty-password" in flags:
> +            if "clear-text-password" in flags:
> +
>                  # chpasswd adds a random salt when running against a
> clear-text password. # For reproducible images, we manually generate
> the password and use the # SOURCE_DATE_EPOCH to generate the salt in
> a deterministic way.
> -                if [ -z "${SOURCE_DATE_EPOCH}" ]; then
> -                    chpasswd_args=""
> -                else
> -                    salt="$(echo "${SOURCE_DATE_EPOCH}" | sha256sum
> -z | cut -c 1-15)"
> -                    password="$(openssl passwd -6 -salt $salt
> "$password")"
> -                fi
> -            fi
> -            printf '%s:%s' "$name" "$password" | sudo chroot
> '${ROOTFSDIR}' \
> -                /usr/sbin/chpasswd $chpasswd_args
> -        fi
> -        if [ "${flags}" != "${flags%*,force-passwd-change,*}" ]; then
> -            echo "Execute passwd to force password change on first
> boot for \"$name\""
> -            sudo -E chroot '${ROOTFSDIR}' \
> -                /usr/bin/passwd --expire "$name"
> -        fi
> -    done
> +                source_date_epoch = d.getVar("SOURCE_DATE_EPOCH") or
> ""
> +                if source_date_epoch:
> +                    command.append("-e")
> +                    salt =
> hashlib.sha256("{}\n".format(source_date_epoch).encode()).hexdigest()[0:15]
> +                    password = crypt.crypt(password,
> "$6${}".format(salt)) +
> +            else:
> +                command.append("-e")
> +
> +            bb.process.run(command, "{}:{}".format(entry,
> password).encode()) +
> +        if "force-passwd-change" in flags:
> +            bb.process.run([*chroot, "/usr/sbin/passwd", "--expire",
> entry]) +
> +
> +ROOTFS_POSTPROCESS_COMMAND += "image_postprocess_accounts"
> +python image_postprocess_accounts() {
> +    image_create_groups(d)
> +    image_create_users(d)
>  }