Don't allow useradd to use fully numeric names

Maybe it should be a warning in the SRUs as opposed to a failure, but I don't have a strong opinion. I'm a bit scared of breaking scripts. But maybe that's a good thing.

I don't have a strong opinion either, but given that scripts would ignore the warnings and the resulting numeric users are going to face random, seemingly unrelated issues thanks to the interaction with systemd, I think I prefer the failure.

FWIW, I've prepared a test version in a PPA[1] which keeps the rules from Debian[2] but prevents the fully numeric names. This is what it looks like:

$ useradd 0
useradd: invalid user name '0'

$ echo $?
3

$ sudo useradd 0c0

$ sudo useradd 0 --badnames

$ cat /etc/passwd | grep ^0
0c0:x:1001:1001::/home/0c0:/bin/sh
0:x:1002:1002::/home/0:/bin/sh

[1] https://launchpad.net/~vtapia/+archive/ubuntu/sf305373
[2] https://salsa.debian.org/debian/shadow/-/blob/master/debian/patches/506_relaxed_usernames

The Ubuntu Security team is +1 on disallowing purely numeric usernames, as they are too easily confused with UIDs.

I think our preference would be to disallow leading numeric digits entirely so that for example, 0x0 and 0o0 would be blocked as well, to try to prevent both user and programmatic confusion.

Probably adduser should also be made consistent with whatever change is made to useradd. The package provided adduser.conf files do have a NAME_REGEX option (in addition to the --force-badname option) but AFAICT is commented out by default (the commented out regex is "^[a-z][-a-z0-9_]*\$" but I'm not sure that's appropriate in a UTF-8 world.)

It would be good to have testcase and documentation for this captured somewhere.

This fix for impish uses sbeattie's suggestion of simply disallowing a numeric character at the beginning of the username. It also includes a test case.

> I think our preference would be to disallow leading numeric digits
> entirely so that for example, 0x0 and 0o0 would be blocked as well,
> to try to prevent both user and programmatic confusion.

Disallowing leading numeric digits entirely would, unfortunately, disable a significant class of valid usernames in conflict with historical usage.

The main motivation in fixing this is that allowing fully-numeric usernames means there is ambiguity in contexts that can reference both uids and usernames and do not have strong typing. Aside from systemd, this is mostly about shells and invocations of various commandline tools; and neither bash nor the tools appear to interpret 0o0 or 0x0 as numbers:

$ id 0o0
id: ‘0o0’: no such user
$ id 0x0
id: ‘0x0’: no such user
$ getent passwd 0x0
$ getent passwd 0o0

Let's please focus on the known problem case of all-numeric usernames. If there are other confirmed security issues with octal/hex representations of numbers, then we should also close those, but it needs a more precise fix than disabling leading digits.

On Wed, Jun 16, 2021 at 09:15:32PM -0000, Steve Langasek wrote:
> Disallowing leading numeric digits entirely would, unfortunately,
> disable a significant class of valid usernames in conflict with
> historical usage.

Admins are still able to hand-edit /etc/passwd, /etc/shadow, and mv
home directory names if they've got a good enough reason to use such
names and trust their software to do the right thing.

> The main motivation in fixing this is that allowing fully-numeric
> usernames means there is ambiguity in contexts that can reference both
> uids and usernames and do not have strong typing. Aside from systemd,
> this is mostly about shells and invocations of various commandline
> tools; and neither bash nor the tools appear to interpret 0o0 or 0x0 as
> numbers:

I was thinking primarily of perl, here:

$ sudo perl -e 'print "muahaa\n" if $< == "0x0";'
muahaa

You could argue that wherever "0x0" came from in this perl program should
have kept track if it received a number or a name, but the language sure
doesn't help.

C examples are less compelling because it has types but the atoi(3)
and strtoul(3) APIs make it very easy to parse something like "2build"
or "4fun" or "0x0" into an integer. (strtol(3) has a nice example.)

> Let's please focus on the known problem case of all-numeric usernames.
> If there are other confirmed security issues with octal/hex
> representations of numbers, then we should also close those, but it
> needs a more precise fix than disabling leading digits.

How strongly do you feel about this? I can see where you're coming from,
but given (a) the escape hatch mechanism to 'break the rules' isn't too
onerous (b) the ease with which brittle code can be written (c) the
simplicity of 'deny leading digit' compared against 'make sure there's at
least one non-digit' or 'make sure there's at least one letter' etc I
prefer the simpler rule.

Thanks

The attachment "lp1927078.debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

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

Heh, a comment in Jawn's debdiff:

    * User/group names must match [a-z_][a-z0-9_-]*[$]

I found period also worked fine:

root@u20:~# useradd 0.0
root@u20:~# getent passwd 0.0
0.0:x:1001:1001::/home/0.0:/bin/sh
root@u20:~# userdel 0.0
root@u20:~# getent passwd 0.0
root@u20:~# exit

I know comments are almost always out of date by the time I read them, but this one seems wronger than usual. :)

@Seth that very incorrect comment is actually part of block of upstream code that is commented out. It doesn't apply to the more relaxed Debian username scheme.

Attached is a patch that disallows fully numeric usernames while still allowing usernames such as 91jawn-smith.

Ah, that explains that.

Would you mind adding tests for a few more usernames?

0root
0
00
0.0
0x0
0-0
0_0
0.o
0xo
0-o
0_o

Thanks

Thanks for looking at this @William - sorry to nitpick but I wonder if rewriting the test as follows could make it a bit easier to parse (at least for me I find this version easier to grok what is being tested for):

if (*name < '1' || *name > '9')

I'd be happy to make that change and add the test cases, but I'm still not sure which patch we landed on. I'm rather indifferent so I will leave the discussion to others before adding those test cases to whichever method we decide.

Given that this is still under discussion I'm going to unsubscribe the ubuntu-sponsors team.

It's fine for us to disallow fully-numeric usernames (including octal and hex syntax).

It would be inappropriate, especially in SRU, to change the policy to restrict other usernames that happen to begin with a digit.

This patch only disallows usernames that are strictly numeric per vorlon's comment above. It also adds more test cases for invalid usernames such as "0123456789" and valid usernames such as "0root" and "0.o". This time I also remembered that '0' is a digit.

Beautiful, thanks for the large range of tests :)

This change disallows floating point and hexadecimal representations of numbers as well as purely numeric, which should be a good compromise. For example, 0x0 is now invalid, as well as 0x123456789 and 0.0, while 0x0x0x0x is considered valid. It also adds these new restrictions to the man page.

This one adds in a check for octal representation and some test cases for octal representation.

Thank you! This one is looking pretty solid overall.
But I think the "hex detection" has some flaws, as it only checks for capital letters and there is a discrepancy between accepting float hex numbers but rejecting float octal numbers.

IMO hex or octal numbers containing a "." (float) should just be considered non-numeric. Should they?

Could you please test for some more edge cases like and make sure they all work (and maybe also octal floats):

"0xDEADBEEF" "0xcafe42" "0xdeadbeef" "0xdead.beef" "0x."

Per our discussion I just removed floating point checks altogether. "0.123" is now considered valid because it is impossible to have a floating point uid or gid so there will be no confusion with floating point numbers. I have added those floating point numbers to the "validUsernames" test, and added a few more cases that are almost numeric, like "0xdeadbeefjawn-smith" to make sure they are seen as valid.

Thanks William, this LGTM now!

I've modified your debian/changelog and the XML/man pages a bit, to account for the removed handling of floating point numbers. And uploaded the package.

$ dput ubuntu ../shadow_4.8.1-1ubuntu9_source.changes
D: Setting host argument.
Checking signature on .changes
gpg: ../shadow_4.8.1-1ubuntu9_source.changes: Valid signature from 5889C17AB1C8D890
Checking signature on .dsc
gpg: ../shadow_4.8.1-1ubuntu9.dsc: Valid signature from 5889C17AB1C8D890
Uploading to ubuntu (via sftp to upload.ubuntu.com):
  Uploading shadow_4.8.1-1ubuntu9.dsc: done.
  Uploading shadow_4.8.1-1ubuntu9.debian.tar.xz: done.
  Uploading shadow_4.8.1-1ubuntu9_source.buildinfo: done.
  Uploading shadow_4.8.1-1ubuntu9_source.changes: done.
Successfully uploaded packages.

https://launchpad.net/ubuntu/+source/shadow/1:4.8.1-1ubuntu9

This bug was fixed in the package shadow - 1:4.8.1-1ubuntu9

---------------
shadow (1:4.8.1-1ubuntu9) impish; urgency=medium

  * Disallow purely numeric usernames. This includes hexadecimal and
    octal syntax. (LP: #1927078)

 -- William 'jawn-smith' Wilson <email address hidden> Thu, 17 Jun 2021 14:35:15 -0500

The attached patch fixes the issue for hirsute.

The attached patch fixes the issue in focal

Thanks, I've sponsored the HH & FF uploads.

Hello Victor, or anyone else affected,

Accepted shadow into hirsute-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/shadow/1:4.8.1-1ubuntu8.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, what testing has been performed on the package and change the tag from verification-needed-hirsute to verification-done-hirsute. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-hirsute. In either case, without details of your testing we will not be able to proceed.

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

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Unsubscribing sponsors.

Hello Victor, or anyone else affected,

Accepted shadow into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/shadow/1:4.8.1-1ubuntu5.20.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, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

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

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

The verification passed for hirsute.

jawn-smith@lvm:~$ apt-cache policy passwd
passwd:
  Installed: 1:4.8.1-1ubuntu8
  Candidate: 1:4.8.1-1ubuntu8.1
  Version table:
     1:4.8.1-1ubuntu8.1 500
        500 http://us.archive.ubuntu.com/ubuntu hirsute-proposed/main amd64 Packages
 *** 1:4.8.1-1ubuntu8 500
        500 http://us.archive.ubuntu.com/ubuntu hirsute/main amd64 Packages
        100 /var/lib/dpkg/status
jawn-smith@lvm:~$ sudo useradd 123
jawn-smith@lvm:~$ grep "^123" /etc/passwd
123:x:1001:1001::/home/123:/bin/sh
jawn-smith@lvm:~$ sudo userdel 123
jawn-smith@lvm:~$ sudo apt install passwd
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
The following packages will be upgraded:
  passwd
1 upgraded, 0 newly installed, 0 to remove and 45 not upgraded.
Need to get 793 kB of archives.
After this operation, 298 kB disk space will be freed.
Get:1 http://us.archive.ubuntu.com/ubuntu hirsute-proposed/main amd64 passwd amd64 1:4.8.1-1ubuntu8.1 [793 kB]
Fetched 793 kB in 0s (1,711 kB/s)
(Reading database ... 188906 files and directories currently installed.)
Preparing to unpack .../passwd_1%3a4.8.1-1ubuntu8.1_amd64.deb ...
Unpacking passwd (1:4.8.1-1ubuntu8.1) over (1:4.8.1-1ubuntu8) ...
Setting up passwd (1:4.8.1-1ubuntu8.1) ...
Processing triggers for man-db (2.9.4-2) ...
jawn-smith@lvm:~$ useradd 123
useradd: invalid user name '123'
jawn-smith@lvm:~$ grep "^123" /etc/passwd
jawn-smith@lvm:~$

The verification passed for focal.

jawn-smith@focal-vm:~$ apt-cache policy passwd
passwd:
  Installed: 1:4.8.1-1ubuntu5.20.04
  Candidate: 1:4.8.1-1ubuntu5.20.04.1
  Version table:
     1:4.8.1-1ubuntu5.20.04.1 500
        500 http://us.archive.ubuntu.com/ubuntu focal-proposed/main amd64 Packages
 *** 1:4.8.1-1ubuntu5.20.04 500
        500 http://us.archive.ubuntu.com/ubuntu focal-updates/main amd64 Packages
        100 /var/lib/dpkg/status
     1:4.8.1-1ubuntu5 500
        500 http://us.archive.ubuntu.com/ubuntu focal/main amd64 Packages
jawn-smith@focal-vm:~$ sudo useradd 123
jawn-smith@focal-vm:~$ grep "^123" /etc/passwd
123:x:1001:1001::/home/123:/bin/sh
jawn-smith@focal-vm:~$ sudo userdel 123
jawn-smith@focal-vm:~$ sudo apt install passwd
Reading package lists... Done
Building dependency tree
Reading state information... Done
The following packages were automatically installed and are no longer required:
  linux-headers-5.8.0-43-generic linux-headers-5.8.0-50-generic linux-hwe-5.8-headers-5.8.0-43 linux-hwe-5.8-headers-5.8.0-50
  linux-image-5.8.0-43-generic linux-image-5.8.0-50-generic linux-modules-5.8.0-43-generic linux-modules-5.8.0-50-generic
  linux-modules-extra-5.8.0-43-generic linux-modules-extra-5.8.0-50-generic
Use 'sudo apt autoremove' to remove them.
The following packages will be upgraded:
  passwd
1 upgraded, 0 newly installed, 0 to remove and 49 not upgraded.
Need to get 799 kB of archives.
After this operation, 1,024 B of additional disk space will be used.
Get:1 http://us.archive.ubuntu.com/ubuntu focal-proposed/main amd64 passwd amd64 1:4.8.1-1ubuntu5.20.04.1 [799 kB]
Fetched 799 kB in 0s (2,027 kB/s)
(Reading database ... 275608 files and directories currently installed.)
Preparing to unpack .../passwd_1%3a4.8.1-1ubuntu5.20.04.1_amd64.deb ...
Unpacking passwd (1:4.8.1-1ubuntu5.20.04.1) over (1:4.8.1-1ubuntu5.20.04) ...
Setting up passwd (1:4.8.1-1ubuntu5.20.04.1) ...
Processing triggers for man-db (2.9.1-1) ...
jawn-smith@focal-vm:~$ sudo useradd 123
useradd: invalid user name '123'
jawn-smith@focal-vm:~$ grep "^123" /etc/passwd
jawn-smith@focal-vm:~$

All autopkgtests for the newly accepted shadow (1:4.8.1-1ubuntu5.20.04.1) for focal have finished running.
The following regressions have been reported in tests triggered by the package:

mysql-8.0/8.0.25-0ubuntu0.20.04.1 (arm64, i386, ppc64el, amd64)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/focal/update_excuses.html#shadow

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

All autopkgtests for the newly accepted shadow (1:4.8.1-1ubuntu8.1) for hirsute have finished running.
The following regressions have been reported in tests triggered by the package:

mysql-8.0/8.0.25-0ubuntu0.21.04.1 (i386, ppc64el, amd64, arm64)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/hirsute/update_excuses.html#shadow

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

I think it would be good to write a release note entry about this change for Ubuntu 21.10 and possibly updating the release notes for Ubuntu 20.04.3 (if these changes make it there in time).

Put a mention about it in the release notes for .3. Will do the same for impish.

The verification of the Stable Release Update for shadow has completed successfully and the package is now being 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.

This bug was fixed in the package shadow - 1:4.8.1-1ubuntu8.1

---------------
shadow (1:4.8.1-1ubuntu8.1) hirsute; urgency=medium

  * Disallow purely numeric usernames. This includes hexadecimal
    octal syntax. (LP: #1927078)

 -- William 'jawn-smith' Wilson <email address hidden> Wed, 14 Jul 2021 16:57:59 -0500

This bug was fixed in the package shadow - 1:4.8.1-1ubuntu5.20.04.1

---------------
shadow (1:4.8.1-1ubuntu5.20.04.1) focal; urgency=medium

  * Disallow purely numeric usernames. This includes hexadecimal
    octal syntax. (LP: #1927078)

 -- William 'jawn-smith' Wilson <email address hidden> Wed, 14 Jul 2021 17:08:18 -0500