useradd crashes if group list contains whitespace

[Colin Walters]

=== Begin SRU Template ===
[Impact]
A specific usage of user data to cloud-init will fail to add a user.

This cloud-config:
  #cloud-config
  users:
    - default
    - name: foobar
      gecos: "My User"
      groups: sudo, adm

Will fail with information in the cloud-init log showing:
2016-12-19 21:39:32,713 - util.py[WARNING]: Failed to create group adm
2016-12-19 21:39:32,713 - util.py[DEBUG]: Failed to create group adm
Traceback (most recent call last):
...
cloudinit.util.ProcessExecutionError: Unexpected error while running command.
Command: ['groupadd', ' adm']
Exit code: 3
Reason: -
Stdout: ''
Stderr: "groupadd: ' adm' is not a valid group name\n"

While changing the last line to the following would work:
      groups: [sudo, adm]

[Test Case]
$ cat > user-data <<"EOF"
#cloud-config
users:
  - default
  - name: foobar
    gecos: "My User"
    groups: sudo, adm
  - name: wark
    groups: [sudo, adm]
EOF

$ release=yakkety
$ name="$release-1354694"

$ lxc launch "ubuntu-daily:$release" "$name" \
     "--config=user.user-data=$(cat user-data)"

$ sleep 10

## Check foobar is in expected groups
$ lxc exec $name -- groups foobar
foobar : foobar adm sudo

$ lxc exec $name -- groups wark
wark : wark adm sudo

$ lxc exec $name -- grep WARN /var/log/cloud-init.log || echo "no warn"
no warn

[Regression Potential]
There are 3 changes in this commit
a.) if 'groups' entry is a string, split on "," and strip pieces
    The most likely path to failure here is if previously a non-string
    (possibly bytes) was being passed in and now will be ignored.
    That seems unlikely and clearly wrong input.

b.) fix and unit tests to explicitly set system=False or no_create_home=True.
    Previously those paths did not test the value of the entry, only the
    presense of the entry.
    This meant that these 2 configs were the same:
      users: {name: bob, system: True}
    and
      users: {name: bob, system: False}

    That bug is fixed here so that 'system: False' is just explicitly
    disabling the '--system' flag to adduser.

c.) debug message cleanup:

   LOG.debug("created group %s for user %s", name, group)
   LOG.debug("created group '%s' for user '%s'", group, name)

[Other Info]
Upstream commit at
  https://git.launchpad.net/cloud-init/commit/?id=ca3ae67211d907b4cfdcd685c0ae4f9530cb7da1

=== End SRU Template ===

See downstream: https://bugzilla.redhat.com/show_bug.cgi?id=1126365

[Colin Walters (walters)]

Hi Scott,

I have now signed the contributor agreement.

[Lars Kellogg-Stedman (larsks)]

I've submitted a patch for this issue, but this is the patch being carried in the Fedora package, rather than a rewrite of what Colin submitted. That is, it's specific to the 'groups' key, rather than introducing a generic mechanism for stripping whitespace from arguments.

[Scott Moser (smoser)]

Marked this back to confirmed.
The fix that was applied in cbf93eb4ae9f [1] did not actually fix the problem.

The user-data to show a bug is:
#cloud-config
users:
  - default
  - name: foobar
    gecos: "My User"
    groups: sudo, adm
  - name: wark
    groups: [sudo, adm]
  - name: wark2
    groups: [sudo, "adm "]

The 'wark2' user is not really necessary, but the change proposed at [2] would normalize that trailing white space out also.

--
[1] https://git.launchpad.net/cloud-init/commit/?id=cbf93eb4ae9fba0797ab4ae7d62bc0d64611fa7e
[2] https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/313458

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "Patch" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

[Launchpad Janitor (janitor)]

This bug was fixed in the package cloud-init - 0.7.8-68-gca3ae67-0ubuntu1

---------------
cloud-init (0.7.8-68-gca3ae67-0ubuntu1) zesty; urgency=medium

  * New upstream snapshot.
    - user-groups: fix bug when groups was provided as string and had spaces
      (LP: #1354694)

 -- Scott Moser <email address hidden> Mon, 19 Dec 2016 12:35:13 -0500

[Scott Moser (smoser)]

This is fixed in cloud-init 0.7.9.

[Brian Murray (brian-murray)]

Hello Colin, or anyone else affected,

Accepted cloud-init into yakkety-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/cloud-init/0.7.8-68-gca3ae67-0ubuntu1~16.10.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Scott Moser (smoser)]

$ lxc launch "ubuntu-daily:$release" "$name" "--config=user.user-data=$(cat user-data)"
$ cat >user-data <<"EOF"
#cloud-config
users:
  - default
  - name: foobar
    gecos: "My User"
    groups: sudo, adm
  - name: wark
    groups: [sudo, adm]
EOF
$ sleep 10

## show failure.
$ lxc exec $name -- groups foobar
groups: 'foobar': no such user
$ lxc exec $name -- groups wark
wark : wark adm sudo

## show info about instance
$ lxc exec $name -- dpkg-query --show cloud-init
cloud-init 0.7.8-49-g9e904bb-0ubuntu1~16.10.1

$ lxc file pull $name/etc/cloud/build.info -
build_name: server
serial: 20170119

## enable proposed, update
$ m=http://archive.ubuntu.com/ubuntu;
$ echo "deb $m $release-proposed main" | lxc file push - $name/etc/apt/sources.list.d/proposed.list
$ lxc exec $name -- sh -c 'apt update -q && apt install cloud-init' </dev/null
$ lxc exec $name -- dpkg-query --show cloud-init
cloud-init 0.7.8-68-gca3ae67-0ubuntu1~16.10.1

# clean up to make it look like first boot.
$ lxc exec $name -- sh -c 'cd /var/lib/cloud; for d in *; do [ "$d" = "seed" ] || rm -vRf "$d"; done'
$ lxc exec $name -- sh -c 'rm -f /var/log/cloud*'
$ lxc restart $name

# show fixed
$ lxc exec $name -- groups foobar
foobar : foobar adm sudo

$ lxc exec $name -- groups wark
wark : wark adm sudo

$ lxc exec $name -- grep WARN /var/log/cloud-init.log || echo no warn
no warn

[Andy Whitcroft (apw)]

The verification of the Stable Release Update for cloud-init has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package cloud-init - 0.7.8-68-gca3ae67-0ubuntu1~16.10.1

---------------
cloud-init (0.7.8-68-gca3ae67-0ubuntu1~16.10.1) yakkety; urgency=medium

  * debian/cherry-pick: add utility for cherry picking commits from upstream
    into patches in debian/patches.
  * New upstream snapshot.
    - mounts: use mount -a again to accomplish mounts (LP: #1647708)
    - CloudSigma: Fix bug where datasource was not loaded in local search.
      (LP: #1648380)
    - when adding a user, strip whitespace from group list
      [Lars Kellogg-Stedman] (LP: #1354694)
    - fix decoding of utf-8 chars in yaml test
    - Replace usage of sys_netdev_info with read_sys_net (LP: #1625766)
    - fix problems found in python2.6 test.
    - OpenStack: extend physical types to include hyperv, hw_veb, vhost_user.
      (LP: #1642679)
    - tests: fix assumptions that expected no eth0 in system. (LP: #1644043)
    - net/cmdline: Consider ip= or ip6= on command line not only ip=
      (LP: #1639930)
    - Just use file logging by default [Joshua Harlow] (LP: #1643990)
    - Improve formatting for ProcessExecutionError [Wesley Wiedenmeier]
    - flake8: fix trailing white space
    - Doc: various documentation fixes [Sean Bright]
    - cloudinit/config/cc_rh_subscription.py: Remove repos before adding
      [Brent Baude]
    - packages/redhat: fix rpm spec file.
    - main: set TZ in environment if not already set. [Ryan Harper]
    - disk_setup: Use sectors as unit when formatting MBR disks with sfdisk.
      [Daniel Watkins] (LP: #1460715)

 -- Scott Moser <email address hidden> Mon, 19 Dec 2016 15:07:12 -0500

[Brian Murray (brian-murray)]

Hello Colin, or anyone else affected,

Accepted cloud-init into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/cloud-init/0.7.9-0ubuntu1~16.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Brian Murray (brian-murray)]

Hello Colin, or anyone else affected,

Accepted cloud-init into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/cloud-init/0.7.9-0ubuntu1~16.04.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Scott Moser (smoser)]

$ release="xenial"
$ name="$release-1354694"
$ cat >user-data <<"EOF"
> #cloud-config
> users:
> - default
> - name: foobar
> gecos: "My User"
> groups: sudo, adm
> - name: wark
> groups: [sudo, adm]
> EOF
$ sleep 10
$ lxc exec $name -- groups foobar
error: not found
$ lxc launch "ubuntu-daily:$release" "$name" "--config=user.user-data=$(cat user-data)"
Creating xenial-1354694
Starting xenial-1354694
$ sleep 10
$ lxc exec $name -- groups foobar
groups: 'foobar': no such user
$ lxc exec $name -- groups wark
wark : wark adm sudo
$ lxc exec $name -- dpkg-query --show cloud-init
cloud-init 0.7.8-49-g9e904bb-0ubuntu1~16.04.4
$ lxc file pull $name/etc/cloud/build.info -
build_name: server
serial: 20170207
$ m=http://archive.ubuntu.com/ubuntu;
$ echo "deb $m $release-proposed main" | lxc file push - $name/etc/apt/sources.list.d/proposed.list
$ lxc exec $name -- sh -c 'apt update -q && apt install cloud-init' </dev/null
...
The following packages will be upgraded:
  cloud-init
1 upgraded, 0 newly installed, 0 to remove and 20 not upgraded.
...

$ lxc exec $name -- dpkg-query --show cloud-init
cloud-init 0.7.9-0ubuntu1~16.04.2
$ lxc exec $name -- sh -c 'cd /var/lib/cloud; for d in *; do [ "$d" = "seed" ] || rm -vRf "$d"; done'

$ lxc exec $name -- sh -c 'rm -f /var/log/cloud*'
$ lxc restart $name
$
$ lxc exec $name -- groups foobar
foobar : foobar adm sudo
$ lxc exec $name -- groups wark
wark : wark adm sudo
$ lxc exec $name -- grep WARN /var/log/cloud-init.log || echo no warn
no warn

[Launchpad Janitor (janitor)]

This bug was fixed in the package cloud-init - 0.7.9-0ubuntu1~16.04.2

---------------
cloud-init (0.7.9-0ubuntu1~16.04.2) xenial-proposed; urgency=medium

  * debian/update-grub-legacy-ec2: fix shell syntax error. (LP: #1662221)

cloud-init (0.7.9-0ubuntu1~16.04.1) xenial-proposed; urgency=medium

  * debian/copyright: update License field to include Apache.
  * debian/update-grub-legacy-ec2: fix to include kernels whose config
    has CONFIG_XEN=y (LP: #1379080).
  * debian/patches/azure-use-walinux-agent.patch: continue relying on
    walinux agent in stable release.
  * New upstream release.
    - doc: adjust headers in tests documentation for consistency.
    - pep8: fix issue found in zesty build with pycodestyle.
    - integration test: initial commit of integration test framework
      [Wesley Wiedenmeier]
    - LICENSE: Allow dual licensing GPL-3 or Apache 2.0 [Jon Grimm]
    - Fix config order of precedence, putting kernel command line over system.
      [Wesley Wiedenmeier] (LP: #1582323)
    - pep8: whitespace fix [Scott Moser]
    - Update the list of valid ssh keys. [Michael Felt]
    - network: add ENI unit test for statically rendered routes.
    - set_hostname: avoid erroneously appending domain to fqdn
      [Lars Kellogg-Stedman] (LP: #1647910)
    - doc: change 'nobootwait' to 'nofail' in docs [Anhad Jai Singh]
    - Replace an expired bit.ly link in code comment. [Joshua Harlow]
    - user-groups: fix bug when groups was provided as string and had spaces
      [Scott Moser] (LP: #1354694)
    - when adding a user, strip whitespace from group list
      [Lars Kellogg-Stedman] (LP: #1354694)
    - fix decoding of utf-8 chars in yaml test
    - Replace usage of sys_netdev_info with read_sys_net
      [Joshua Harlow] (LP: #1625766)
    - fix problems found in python2.6 test. [Joshua Harlow]
    - Just use file logging by default [Joshua Harlow] (LP: #1643990)
    - Improve formatting for ProcessExecutionError [Wesley Wiedenmeier]
    - flake8: fix trailing white space
    - Doc: various documentation fixes [Sean Bright]
    - cloudinit/config/cc_rh_subscription.py: Remove repos before adding
      [Brent Baude]
    - packages/redhat: fix rpm spec file.
    - main: set TZ in environment if not already set. [Ryan Harper]

 -- Scott Moser <email address hidden> Mon, 06 Feb 2017 16:18:28 -0500

[James Falcon (falcojr)]

Tracked in Github Issues as https://github.com/canonical/cloud-init/issues/2473