Opened 9 years ago

Closed 9 years ago

#2152 closed defect (fixed)

Registration fails if user name contains ":"

Reported by: bernie Owned by: anurag_seeta
Priority: Unspecified by Maintainer Milestone: Unspecified
Component: Sugar Version: Git as of bugdate
Severity: Unspecified Keywords: dextrose sugar-love
Cc: martin.langhoff, tch, manusheel, smparrish, dsd Distribution/OS: Unspecified
Bug Status: Unconfirmed

Description (last modified by martin.langhoff)

The schoolserver idmgr daemon passes the user name provided by Sugar directly to useradd to create new UNIX users.

The character ":" isn't allowed in /etc/passwd and makes the registration fail (without providing a clear explanation given, this is another bug imho).

For maximum version interoperability, I suggest *both* Sugar and the XS should convert illegal characters to "_".

There is no "schoolserver" component so left it as 'Sugar'.

Change History (15)

comment:1 Changed 9 years ago by bernie

  • Cc tch added

comment:2 Changed 9 years ago by anurag_seeta

Can you please provide with more detailed steps in order to reproduce the bug on the developers' end

comment:3 Changed 9 years ago by bernie

  1. On first boot (or from the "About Me" control panel), type in a name containing the ':' character.
  2. go to the Buddy menu and select Register (on recent Dextrose releases, you can register multiple times. On other builds, you might have to wipe your profile first)
  3. Registration will fail without a clear explanation given. Looking at the idmgr logs on the schoolserver reveals the actual issue with ':' being an invalid character for any field in /etc/passwd.

comment:4 Changed 9 years ago by bernie

The proposed fixes for this bug are:

  1. refusing the ":" character in the name input field of the first boot and About Me control panel.
  2. converting each ":" to "_" when sending the registration to the schoolserver.
  3. in idmgr, convert ":" to "_" as well.

Implementing all three fixes will ensure robust interoperability across different versions of Sugar and the XS.

comment:5 Changed 9 years ago by seeta

  • Owner changed from tomeu to anurag_seeta
  • Status changed from new to assigned

comment:6 Changed 9 years ago by seeta

  • Cc manusheel added

comment:7 Changed 9 years ago by bernie

  • Keywords sugar-love added

comment:8 Changed 9 years ago by smparrish

  • Cc smparrish added

comment:9 Changed 9 years ago by smparrish

  • seeta_dev set to Dipankar

comment:10 follow-up: Changed 9 years ago by dsd

  • Cc dsd added

I think it's perfectly reasonable that Sugar accepts a colon and even sends it during registration -- Sugar doesn't know that the other end is going to put that in a GECOS field of a unix account. An alternative idmgr implementation could do something completely different, or have a different set of illegal characters.

So, I think this problem should be confirmed to being an idmgr bug and we should fix it there and only there. No sugar changes. I also suspect that there's some way of including a : in a GECOS by escaping it, but I'm only guessing.

comment:11 in reply to: ↑ 10 Changed 9 years ago by bernie

Replying to dsd:

So, I think this problem should be confirmed to being an idmgr bug and we should fix it there and only there. No sugar changes. I also suspect that there's some way of including a : in a GECOS by escaping it, but I'm only guessing.

I agree, the bug is in idmgr (or in the protocol not specifying what characters are accepted).

Nevertheless, since no new schoolserver releases are imminent, this bug is likely to stay in production for many years, making registration fail mysteriously (because Sugar isn't into the habit of telling users what went wrong, to avoid "confusing" them).

This is why I think it would be good to work it around on the Sugar side as well. It costs only one line of code and helps mitigate solve the actual issue NOW.

comment:12 Changed 9 years ago by dsd

I disagree, I think updating a handful of school servers is a much simpler solution than upgrading all the computers in 1 deployment.

comment:13 follow-up: Changed 9 years ago by dsd

Also, we don't actually need an XS release, we just need a rpm pushed to the repositories. Then its as simple as "yum update" which is a documented procedure.

An XS-0.6.1 release is tentatively planned for this week. I assume you've already coordinated that it's fixed in idmgr upstream.

comment:14 in reply to: ↑ 13 Changed 9 years ago by bernie

Replying to dsd:

Also, we don't actually need an XS release, we just need a rpm pushed to the repositories. Then its as simple as "yum update" which is a documented procedure.
An XS-0.6.1 release is tentatively planned for this week.

Then I agree with you, there's no need to fix it also in Sugar.

I assume you've already coordinated that it's fixed in idmgr upstream.

Not me, maybe Martin?

comment:15 Changed 9 years ago by martin.langhoff

  • Description modified (diff)
  • Resolution set to fixed
  • Status changed from assigned to closed

Right -- for a moment I worried that we were missing out on some input sanitization but that's not the case. Not only Bobby Tables, but Bobby Backticks and friends all register ok. Only bobby Smiley fails, because ':' is the filed separator in /etc/passwd and there is no escape sequence for it.

This gets cured with http://dev.laptop.org/git/projects/idmgr/commit/?id=2384a1b0b578c3fc9eeaa5e8b7d9c1617ca9f0eb which just removes it.

Which is rolled in a nice idmgr rpm for the 0.6 series.

yum --enablerepo=olpcxs-testing install idmgr

should bring idmgr-0.7.22 (or newer).

Note: See TracTickets for help on using tickets.