X2Go Bug report logs - #1229
x2go rejects usernames starting with digits, incorrectly

Full log

🔗 View this message in rfc822 format

MIME-Version: 1.0
X-Mailer: MIME-tools 5.507 (Entity 5.507)
X-Loop: owner@bugs.x2go.org
From: owner@bugs.x2go.org (X2Go Bug Tracking System)
Subject: Bug#1229 closed by X2Go Release Manager X2Go Release Manager
 <git-admin@x2go.org> (X2Go issue (in src:x2goserver) has been marked as
 closed)
Message-ID: <handler.1229.c.15101501267411.notifdone@bugs.x2go.org>
References: <20171108140446.3CF645DAD3@ymir.das-netzwerkteam.de>
X-X2go-PR-Keywords: pending
X-X2go-PR-Message: they-closed 1229
X-X2go-PR-Package: x2goserver
X-X2go-PR-Source: x2goserver
Date: Wed, 08 Nov 2017 14:10:06 +0000
Content-Type: multipart/mixed; boundary="----------=_1510150206-8570-0"

[Message part 1 (text/plain, inline)]

This is an automatic notification regarding your Bug report
which was filed against the x2goserver package:

#1229: x2go rejects usernames starting with digits, incorrectly

It has been closed by X2Go Release Manager X2Go Release Manager <git-admin@x2go.org>.

Their explanation is attached below along with your original report.
If this explanation is unsatisfactory and you have not received a
better one in a separate message then please contact X2Go Release Manager X2Go Release Manager <git-admin@x2go.org> by
replying to this email.


-- 
X2Go Bug Tracking System
Contact owner@bugs.x2go.org with problems

[Message part 2 (message/rfc822, inline)]

From: X2Go Release Manager X2Go Release Manager <git-admin@x2go.org>

To: 1229-submitter@bugs.x2go.org

Cc: control@bugs.x2go.org, 1229@bugs.x2go.org

Subject: X2Go issue (in src:x2goserver) has been marked as closed

Date: Wed, 8 Nov 2017 15:04:46 +0100 (CET)

close #1229
thanks

Hello,

we are very hopeful that X2Go issue #1229 reported by you
has been resolved in the new release (4.0.1.21) of the
X2Go source project »src:x2goserver«.

You can view the complete changelog entry of src:x2goserver (4.0.1.21)
below, and you can use the following link to view all the code changes
between this and the last release of src:x2goserver.

    http://code.x2go.org/gitweb?p=x2goserver.git;a=commitdiff;h=b8cd8ad873763b546632b8b1ee2762e058da75f4;hp=2c504692966fa2ac456961c2fe561f53c33536b5

If you feel that the issue has not been resolved satisfyingly, feel
free to reopen this bug report or submit a follow-up report with
further observations described based on the new released version
of src:x2goserver.

Thanks a lot for contributing to X2Go!!!

light+love
X2Go Git Admin (on behalf of the sender of this mail)

---
X2Go Component: src:x2goserver
Version: 4.0.1.21-0x2go1
Status: RELEASE
Date: Wed, 08 Nov 2017 15:01:58 +0100
Fixes: 1151 1153 1198 1229 1230
Changes:
 x2goserver (4.0.1.21-0x2go1) RELEASED; urgency=medium
 .
   [ Mihai Moldovan ]
   * New upstream version (4.0.1.21):
     - x2goserver/sbin/x2gocleansessions: don't overzealously try to remove old
       nxagent sockets. If the nxagent process already vanished, simply forget
       the remembered session. Otherwise we might delete sockets that have been
       re-assigned to other sessions in the meantime.
     - x2goserver/bin/x2gostartagent: convert X2GO_PORT searching algorithm to
       pure bash and let script fail if no display port is available.
     - x2goserver/sbin/x2gocleansessions: fix syntax error introduced in last
       change to this file.
     - x2goserver/bin/x2gosetkeyboard: make compatible with changes in
       Arctica's nxagent. Also change to an all-bash algorithm. Fixes: #1151.
     - x2goserver/bin/x2gosetkeyboard: fix a typo that caused the script to not
       work properly - with or without Arctica's nx-libs.
     - x2goserver/bin/x2gosetkeyboard: rename ${setxkbcomp_opts} to
       ${setxkbmap_opts}, that's what it really is.
     - x2goserver/bin/x2gosetkeyboard: setxkbmap expects a parameter called
       "-option", but nxagent uses the key "options". Make setxkbmap happy.
     - x2goserver/bin/x2gosetkeyboard: $BASH_REMATCH is tricky, actually fetch
       the first match. In the $BASH_REMATCH array, index 0 contains the *full*
       match, while indices 1+ only contain the partial matches. We're only
       interested in the first partial match.
     - x2goserver/bin/x2gosetkeyboard: only match up the first ending quote
       character in a non-greedy way. This will not support nested quotes
       within values, but we probably don't need this feature anyway.
     - x2goserver/bin/x2gostartagent: try to enable lingering via systemd's
       loginctl utility before calling x2goagent. Fixes: #1198.
     - x2goserver-xsession/etc/Xsession: support Devuan just like Debian, give
       useful error message in case the OS is unknown.
     - x2goserver/bin/x2gostartagent: fetch hostname via "hostname -s" and do
       not rely on the HOSTNAME variable. The latter is only set automatically
       by bash if it's not already part of the environment. We might get
       "garbage" in this way (and one user actually did.)
     - x2goserver/bin/x2golistshadowsessions: whitespace only.
     - x2goserver/sbin/x2gocleansessions: close syslog at program exit.
     - x2goserver/sbin/x2gocleansessions: whitespace only.
     - x2goserver/bin/x2golistshadowsessions: copy and use a few needed utility
       functions from x2gocleansessions.
     - x2goserver/bin/x2golistshadowsessions: mark unavailable functionality
       with FIXME comments.
     - x2goserver-xsession/etc/Xsession: add support for OS RT via
       /etc/os-rt-release (file needs to be created by upstream first, bug
       pending.)
     - x2goserver/{bin/x2golistshadowsessions,
       lib/x2go{dbwrapper.pm,sqlitewrapper.pl}}: backport listshadowsessions*
       DB functions and unmark them as FIXME entries.
     - x2goserver/lib/x2go{sqlitewrapper.pl,utils.pm}: allow arbitrary-length
       user names, user names starting with digits and drop $ as a valid user
       name character. Fixes: #1229.
     - x2goserver/bin/x2goruncommand: use dbus-run-session to start a new dbus
       user session if available. Only for full desktop sessions. Backported
       from a Debian patch.
     - x2goserver/bin/x2goresume-session: backport HOSTNAME changes from
       x2goserver/bin/x2gostartagent.
     - x2goserver/bin/x2go{resume-session,startagent}: catch errors while
       inserting values into database more gracefully. Fixes: #1230.
     - x2goserver/bin/x2gostartagent: write hostname errors to stderr to make
       X2Go Client (and maybe PyHoca?) fail.
     - x2goserver/{bin/x2go{runcommand,suspend-session},
       sbin/x2gocleansessions}: remove hopefully redundant HOSTNAME export
       commands.
     - x2goserver/etc/x2goagent.options: remove file, moved to x2goagent.
     - x2goserver/Makefile: actually unreference x2goagent.options file as
       well.
     - x2goserver/bin/x2go{resume-session,startagent}: revert error catching
       while inserting values into database. The current code structure relies
       on ignoring failures and retrying until an operation succeeded, which
       may eventually happen or just as well may never happen. We need
       something smarter here.
     - x2goserver/bin/x2gostartagent: log x2goagent startup command to syslog
       in debug mode.
   * x2goserver.spec:
     - Add mandatory perl-generators Build-Requires as per
       https://fedoraproject.org/wiki/Changes/Build_Root_Without_Perl
     - Correct usage of mime and desktop database upgrade scriptlets and
       dependencies. Fedora mandates to not depend upon shared-mime-info, even
       not for the scriplets. Additionally, FC24 has deprecated usage of the
       mime database update scriptlet and FC25 has deprecated usage of the
       desktop database update scriptlet. (Open)SUSE mandates to pull in
       shared-mime-info and desktop-file-utils as post/postun scriptlet
       dependencies and provides special macros since 11.4. Older versions are
       handled via the usual commands.
     - Simplify systemd usage a bit and make sure that it's restarted on Fedora
       and RHEL-based distros. FIXME: Not enabled by default on FC/RHEL/*SuSE.
     - Use Suggests instead of Recommends, as an even weaker dependency. We can
       live just fine without printing or fmbindings and no functions of the
       x2goserver package as well will be impacted. The DEB control file also
       uses "Suggests" for this.
     - Fix RPM spec syntax error in shared-mime-info dependency section.
     - Fix %{?fedora} macro usage.
     - Add the correct package name to the %posttrans scriptlet, we only need
       it for x2goserver-fmbindings.
     - Make sure that we don't go into branches just because a macro is not
       defined on our platform.
     - Work around a bug in SuSE's mime DB update script...
     - Make sure that there's always at least one (nil) command in the if
       command list.
     - Break older SLES builds on purpose to see what branch is *actually*
       taken. Will be reverted shortly.
     - Fix typo in %if condition (0%?{?fedora} instead of 0%{?fedora}.)
     - Contrary to other information that says "brackets" and consistent with
       RPM's source code, grouping is done via "(" and ")" in conditions.
     - More platform-specific condition fixup.
     - Pull in shared-mime-info as a build requirement only on *SuSE and add
       comment regarding why desktop-file-utils is always needed.
     - Also add a trans(action)-requirement on shared-mime-info for older
       RHEL-based distros.
     - RPM spec does not allow %elif or %elseif, which lead to all weird kinds
       of errors. Split these up into nested conditions.
     - There is no %trans scriptlet, we're using %posttrans, so the dependency
       should be specified as Requires(posttrans) as well.
     - Pull in dbus-run-session if possible. Only available on Fedora, SLE12+
       and OpenSuSE 13+.
     - Pull in x2goagent >= 3.5.0.33 explicitly, first version that reflects
       the x2goagent.options file move.
   * debian/po:
     - Tiny fixup on author name.
   * debian/control:
     - Whitespace fixes.
     - Add BD on dh-systemd. We'll need it for systemd integration.
     - Depend upon dbus for dbus-run-session.
     - Pull in x2goagent >= 3.5.0.33 explicitly, first version that reflects
       the x2goagent.options file move.
   * debian/rules:
     - Call the systemd addon to process and install the systemd service file.
     - Don't fail if we don't have the systemd addon. Systems that don't have
       that will likely not use systemd in the first place.
 .
   [ Martti Pitkänen ]
   * debian/po:
     - Translate DebConf templates to Finnish.
 .
   [ Orion Paplowski ]
   * x2goserver.spec:
     - Requires(post) does not imply Requires, and missing ones on grep and the
       SQLite perl module.
     - Fedora >= 21 and (Open)SUSE >= 11 now allow Recommends.
     - perl(:MODULE_COMPAT...) is only needed for perl modules. Drop from
       x2goserver-printing and x2goserver-xsession.
     - Delete .packlist which may be in different locations.
     - Sort some %files entries.
 .
   [ Mike Gabriel ]
   * New upstream version (4.0.1.21):
     - x2goserver/bin/x2goruncommand: stop exporting LD_LIBRARY_PATH when using
       Arctica's nx-libs and its new Xinerama feature. Fixes: #1153.
     - Make x2goruncommand aware of the LXQt desktop shell.
     - x2goserver/x2gosqlitewrapper.c: fix implicit declaration of execv().
     - x2goserver/: backport x2golistshadowsessions.

[Message part 3 (message/rfc822, inline)]

From: "Norman Gray" <gray@nxg.name>

To: submit@bugs.x2go.org

Subject: x2go rejects usernames starting with digits, incorrectly

Date: Fri, 27 Oct 2017 17:51:27 +0100

Package: x2goserver
Version: 4.0.1.20

At present, x2goserver sanitises usernames with a regexp in x2goutils.pm 
and in x2gosqlitewrapper.pl (same in both places).  That's:

    if ($string =~ 
/^([a-zA-Z\_][a-zA-Z0-9\_\-\.\@]{0,47}[\$]?)\-([\d]{2,4})\-([\d]{9,12})\_[a-zA-Z0-9\_\-\.]*\_dp[\d]{1,2}$/) 
{

A username of, eg, '1234567x' fails this test, and the x2go session 
fails to start.  This is a valid username on CentOS (which is the OS I'm 
using in this case, but CentOS is far from unique here), therefore such 
a username should _not_ be rejected.

I have verified that the above code is indeed the source of my login 
problems, since if I hack the two files above, to have the regexp start 
with [a-zA-Z0-9\_], then my users can log in without difficulty.  This 
hacking is obviously not a great solution.

An alternative test would be to use getpwent(3).  This would verify that 
the proffered username is valid, absolutely authoritatively, without 
making any assumptions about what is or isn't valid on the current 
platform.  x2go should not second-guess getpwent(3).

In Perl terms, the above test could be replace with:

    $uid = getpwnam($string);
    if ($uid) {
        # username is OK
    } else {
        # user $string does not exist
    }

Note that the test may in fact be redundant, since if this code is being 
run, then the corresponding user has already logged on to the system, so 
that the username has already been verified as valid and existing.

Other observations:

  * If the system (or specifically getpwent) regards a given username 
as valid, then it will be valid for calls to other local library calls.  
If this were not the case, that would be a major system bug on that 
platform.

  * POSIX/Single Unix says of the username simply "To be portable 
across systems conforming to POSIX.1-2008, the value is composed of 
characters from the portable filename character set. The <hyphen-minus> 
character should not be used as the first character of a portable user 
name." (see <http://pubs.opengroup.org/onlinepubs/9699919799/>, 
paragraph 3.437)

  * The Debian useradd(8) page recommends something matching 
/^[a-z_][a-z0-9_-]*$/, but goes on to say "On Debian, the only 
constraints are that usernames must neither start with a dash ('-') nor 
contain a colon (':') or a whitespace (space: ' ', end of line: '\n', 
tabulation: '\t', etc.). Note that using a slash ('/') may break the 
default algorithm for the definition of the user's home directory." (see 
eg <https://www.unix.com/man-page/linux/8/useradd/>)

  * The corresponding RedHat/CentOS manpage doesn't even include that, 
and instead says only "Usernames may only be up to 32 characters long."  
FreeBSD is similarly laid-back about the username.

  * The GNU Coreutils manual 
<https://www.gnu.org/software/coreutils/manual/coreutils.html#Disambiguating-names-and-IDs> 
which explicitly acknowledges that an all-digits username is legitimate, 
and describes how the coreutils resolve the potential ambiguity.

  * It may have been true in the past that some unixes objected to 
all-digits usernames.  I personally am not aware of any current unixes 
which do so.

  * It is not an option to change the usernames ('1234567x') on this 
system, since they are widely deployed in other systems.  Also, they're 
valid username as far as the local system is concerned.

This issue was discussed on the user list a little while ago 
<http://lists.x2go.org/pipermail/x2go-user/2015-April/003161.html> 
(that's what gave me the aha!).  There, Mihai Moldovan said "That's 
non-standard-compliant and you're basically on your own when doing 
"funky stuff"."  To be clear, I agree such usernames are less than 
ideal, but I don't think they count as funky or non-compliant.

The issue was discussed on the x2go-dev list more recently, starting at 
<http://lists.x2go.org/pipermail/x2go-dev/2017-October/012140.html>

Best wishes,

Norman


-- 
Norman Gray  :  https://nxg.me.uk

Send a report that this bug log contains spam.