From unknown Sun Apr 12 06:19:14 2026 X-Loop: owner@bugs.x2go.org Subject: Bug#1229: [X2Go-Dev] x2go rejects usernames starting with digits, incorrectly Reply-To: Mihai Moldovan , 1229@bugs.x2go.org Resent-From: Mihai Moldovan Resent-To: x2go-dev@lists.x2go.org Resent-CC: X2Go Developers X-Loop: owner@bugs.x2go.org Resent-Date: Sat, 28 Oct 2017 08:10:02 +0000 Resent-Message-ID: Resent-Sender: owner@bugs.x2go.org X-X2Go-PR-Message: followup 1229 X-X2Go-PR-Package: x2goserver X-X2Go-PR-Keywords: Received: via spool by 1229-submit@bugs.x2go.org id=B1229.150917817820126 (code B ref 1229); Sat, 28 Oct 2017 08:10:02 +0000 Received: (at 1229) by bugs.x2go.org; 28 Oct 2017 08:09:38 +0000 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on ymir.das-netzwerkteam.de X-Spam-Level: X-Spam-Status: No, score=-2.0 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,T_SPF_HELO_TEMPERROR,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.1 Received: from localhost (localhost [127.0.0.1]) by ymir.das-netzwerkteam.de (Postfix) with ESMTP id D27845DAD1 for <1229@bugs.x2go.org>; Sat, 28 Oct 2017 10:09:32 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at ymir.das-netzwerkteam.de Received: from ymir.das-netzwerkteam.de ([127.0.0.1]) by localhost (ymir.das-netzwerkteam.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id a4d9lExujs03 for <1229@bugs.x2go.org>; Sat, 28 Oct 2017 10:09:17 +0200 (CEST) Received: from mail.ionic.de (ionic.de [87.98.244.45]) by ymir.das-netzwerkteam.de (Postfix) with ESMTP id 990575DA81 for <1229@bugs.x2go.org>; Sat, 28 Oct 2017 10:09:16 +0200 (CEST) Received: from [10.20.40.15] (178.162.222.163.adsl.inet-telecom.org [178.162.222.163]) by mail.ionic.de (Postfix) with ESMTPSA id 60DE64F00427; Sat, 28 Oct 2017 10:09:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=ionic.de; s=default; t=1509178152; bh=Jh2hilXzw0I24xA+LkEUFoO6DVBRiyykLTMDEF0KqsM=; h=Subject:To:References:From:Date:In-Reply-To:From; b=f1w1SW5D5B1yEAee2NJMiHBBhdNx1cbC5ql3CaTU28OdDbezv57Ksqx1d/uh1YFbc d47nAkkjU7YCIH3xd2QTSjHikXAy0e54r+4boiFUjHmRJDVDyAacczxEIu7lGG9cls odpQZdoFKpQBvxeaD2SfoyleKggId1+RYpDRDk/Q= To: Norman Gray , 1229@bugs.x2go.org References: <2862B49A-8FA8-4EF0-AB61-AC0B863EB3ED@nxg.name> From: Mihai Moldovan Message-ID: <5ef891d1-4119-59e0-b503-ee301f539ba5@ionic.de> Date: Sat, 28 Oct 2017 10:09:11 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <2862B49A-8FA8-4EF0-AB61-AC0B863EB3ED@nxg.name> Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="ri0HblFcsaW4UfBn6Lp2d9ElDI4H1oBLP" This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --ri0HblFcsaW4UfBn6Lp2d9ElDI4H1oBLP Content-Type: multipart/mixed; boundary="GbIG09q07PrAscEvlNjOkjRS3s4Ho2MCI"; protected-headers="v1" From: Mihai Moldovan To: Norman Gray , 1229@bugs.x2go.org Message-ID: <5ef891d1-4119-59e0-b503-ee301f539ba5@ionic.de> Subject: Re: [X2Go-Dev] x2go rejects usernames starting with digits, incorrectly References: <2862B49A-8FA8-4EF0-AB61-AC0B863EB3ED@nxg.name> In-Reply-To: <2862B49A-8FA8-4EF0-AB61-AC0B863EB3ED@nxg.name> --GbIG09q07PrAscEvlNjOkjRS3s4Ho2MCI Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 10/27/2017 06:51 PM, Norman Gray wrote: > At present, x2goserver sanitises usernames with a regexp in x2goutils.p= m=20 > and in x2gosqlitewrapper.pl (same in both places). [...] Just to make it clear - we're not really "validating user names". I could= n't care less about the user name as such - it's the system's responsibility = to deal with user names and if users managed to login, we can assume the user nam= e is valid, like you've already written on the -dev mailing list and here. What we really do in this part of code is validating a session ID, which = happens to contain a user name. Sadly, as such, what we see as a user name must b= e correctly represented in order to check the session ID. Generally, a session ID should look like that: --_st_dp We want to make sure that this ID is valid and, further, that specific pa= rts can be extracted from it. > A username of, eg, '1234567x' fails this test, and the x2go session=20 > fails to start. This is a valid username on CentOS (which is the OS I'= m=20 > using in this case, but CentOS is far from unique here), therefore such= =20 > a username should _not_ be rejected. Okay, so we want to accept numeric and user names starting with digits. > An alternative test would be to use getpwent(3). This would verify tha= t=20 > the proffered username is valid, absolutely authoritatively, without=20 > making any assumptions about what is or isn't valid on the current=20 > platform. x2go should not second-guess getpwent(3). This is probably appropriate for other checks, yes, if we really need to = make sure that a user name is valid on the system. > Note that the test may in fact be redundant, since if this code is bein= g=20 > run, then the corresponding user has already logged on to the system, s= o=20 > that the username has already been verified as valid and existing. In theory it's redundant. But there is a possibility that we are reading = garbage data, where ever that might come from. Any bug (including our scripts mes= sing up splitting up something, or inserting something invalid into the database = and reading it again later) could trigger such a situation, so IMHO validatio= n of input strings is really not redundant. > * POSIX/Single Unix says of the username simply "To be portable=20 > across systems conforming to POSIX.1-2008, the value is composed of=20 > characters from the portable filename character set. The = =20 > character should not be used as the first character of a portable user = > name." (see ,=20 > paragraph 3.437) So, hyphen is prohibited as the first character. Also, SUS recommends (bu= t does not enforce) using the portable filename character set[0] only for portab= ility, which is restricted to [A-Za-z0-9._-]. Specifically, this does not includ= e any special characters like umlauts, accented characters or generally any oth= er Unicode character. > * The Debian useradd(8) page recommends something matching=20 > /^[a-z_][a-z0-9_-]*$/, but goes on to say "On Debian, the only=20 > constraints are that usernames must neither start with a dash ('-') nor= =20 > contain a colon (':') or a whitespace (space: ' ', end of line: '\n',=20 > tabulation: '\t', etc.). Note that using a slash ('/') may break the=20 > default algorithm for the definition of the user's home directory." (se= e=20 > eg ) This is a bit stricter than the SUS definition (ignoring the portability recommendation). If taken at face value, Debian allows any Unicode charac= ter but the restricted ones. Interestingly, the recommended matching regexp doesn= 't include uppercase characters and, arguably more interestingly, doesn't al= low a user name to start with a digit (which would be problematic for you). > * The corresponding RedHat/CentOS manpage doesn't even include that,= =20 > and instead says only "Usernames may only be up to 32 characters long."= =20 > FreeBSD is similarly laid-back about the username. This is interesting as well, since it's the first document that mentions = a maximum length. If interpreted directly, the previous documents did not r= estrict the length (unless you haven't pasted some information relating to the st= ring length). Currently, we seem to limit the user name to a length of at least one to = 48 characters. Not sure where this limit is coming from, but it's higher tha= n what RHEL/CentOS documents. Specifically, if the 32 characters limit is a hard= one, parsing longer user names than this successfully would be invalid. Then a= gain, I'm not inclined to implement per-system limits, so I'd rather go with th= e most restricted set... In the current code, for some reason we are also allowing the characters = $ and @, if they are not the first characters. I'm not sure why that is, but al= lowing @ might be a consequence of multi-server support (i.e., that @ acts a sep= arator for the host name.) Also, a trailing character of $ is allowed, which ram= ps up the section to 49 characters in total. Going through the history, I can see that previously, a 32 (or 33) charac= ter limit was enforced, but changed to 48 (or 49) as part of this bug report:= https://bugs.x2go.org/cgi-bin/bugreport.cgi?bug=3D574 The @ character indeed has been added to allow email-address-like user na= mes as part of https://bugs.x2go.org/cgi-bin/bugreport.cgi?bug=3D573 Allowing $ as trailing characters has been part from the start, though I honestly don't understand why. So, with all that, the most unrestrained option would be to use /^[^-].*/= for the user name part. If taking the SUS recommendation for portable user names into account, th= at would change to /^[A-Za-z0-9._][A-Za-z0-9._-]*/. If also applying the RHEL restriction of 32 characters, we'd come out at /^[A-Za-z0-9._][A-Za-z0-9._-]{0,31}/. My initial idea was to just use the last regexp, but this would break set= ups with user@domain user names or user names longer than 32 characters. In theory, /^[A-Za-z0-9._][A-Za-z0-9._-@]*/ should be more liberal, not r= estrict the length, allow portable user names and expand on that by allowing domain-based user names as well. I'd drop the trailing $. Mihai [0] http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#t= ag_03_282 --GbIG09q07PrAscEvlNjOkjRS3s4Ho2MCI-- --ri0HblFcsaW4UfBn6Lp2d9ElDI4H1oBLP Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIzBAEBCgAdFiEEbhHQj3UzgcdE8cg8H9Yu2W4lOocFAln0OycACgkQH9Yu2W4l OofslxAArmBF5yqhZpZJOkdZKi2LjUc5KbybViS4xhQeCg6ylHqVJXEvtM5Q461x hfRaDBoom6YToeOY0Bn75c9Q+dfGzvJC/2wWBWvlev0pwEj0+ShvNdhPk5s5OLPB C3714KxbiI75PlJwG2FLSei/6adX6BL6AJjf3Ibsp/w62GIq6HjKGsu2C0e7yQiR 8KsoeX+PDvqdQGCBi4l0HP1ipdTXCf3eI0CsS/yHsWDTkJk9kmTmJbBbM4gjBSUC 9KEvPvZb2h5XWf204fA5aNWO8qcB5u4uKWZrUU+kd6vmbp2hvv9EOsmhRnqJ0NGc 4iCVgCbi1tVUHYXQZIGh53gj+89mqbQcA6TDAd+UwIvwsBSpcIO8BXWoVAd4kRXp 8ppbXTiEAxPPGcQO3hnNm5dbndIBPb2Yo7JMfcd9FIplKQde53i7uUjjLNor3zFD tr1ZA4YGML/qwY7nlbeR2SvvbWRuEBthVrykLfZ5Z0txDZ66a0cLqEDGOv8Txqrn xPS8h2Q/ZjchRmQhhRozwEP9asdL990cyMPM3tqwlzsSDIH7MsQJljBtuROQLlpy tA6MN+yLyZrCNBJEVNv+1KViZK+s+kRtbPLxFxSkuFDLHaEWi1NED8XxJIsvxSlo nuzowmsfdRmH1djbiJPBwQ/+6TYvF+czcQRCsUmpCH+bevqNQX8= =cIBF -----END PGP SIGNATURE----- --ri0HblFcsaW4UfBn6Lp2d9ElDI4H1oBLP--