Bug#1099470: colord: upgrade to colord 1.4.7-3 fails: The user `colord' already exists, but is not a system user.

[Vincent Lefevre]

Package: colord
Version: 1.4.7-3
Severity: serious

When upgrading colord with aptitude:

Setting up colord (1.4.7-3) ...
warn: The home dir /var/lib/colord you specified already exists.

fatal: The user `colord' already exists, but is not a system user. Exiting.
dpkg: error processing package colord (--configure):
installed colord package post-installation script subprocess returned error exit status 13
[...]
Errors were encountered while processing:
colord
[...]
E: Sub-process /usr/bin/dpkg returned an error code (1)
Setting up colord (1.4.7-3) ...
warn: The home dir /var/lib/colord you specified already exists.

fatal: The user `colord' already exists, but is not a system user. Exiting.
dpkg: error processing package colord (--configure):
installed colord package post-installation script subprocess returned error exit status 13
Errors were encountered while processing:
colord

-- System Information:
Debian Release: trixie/sid
APT prefers unstable-debug
APT policy: (500, 'unstable-debug'), (500, 'stable-updates'), (500, 'stable-security'), (500, 'stable-debug'), (500, 'proposed-updates-debug'), (500, 'unstable'), (500, 'testing'), (500, 'stable'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 6.11.10-amd64 (SMP w/12 CPU threads; PREEMPT)
Kernel taint flags: TAINT_PROPRIETARY_MODULE, TAINT_OOT_MODULE, TAINT_UNSIGNED_MODULE
Locale: LANG=C.UTF-8, LC_CTYPE=C.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages colord depends on:
ii acl 2.3.2-2+b1
ii adduser 3.143
ii colord-data 1.4.7-3
ii dconf-gsettings-backend [gsettings-backend] 0.40.0-5
ii libc6 2.41-3
ii libcolord2 1.4.7-3
ii libcolorhug2 1.4.7-3
ii libdbus-1-3 1.16.2-1
ii libglib2.0-0t64 2.83.5-1
ii libgudev-1.0-0 238-6
ii libgusb2 0.4.9-1+b1
ii liblcms2-2 2.16-2
ii libpolkit-gobject-1-0 126-2
ii libsane1 1.3.1-1
ii libsqlite3-0 3.46.1-1
hi libsystemd0 256.7-1
ii polkitd 126-2

colord recommends no packages.

Versions of packages colord suggests:
pn colord-sensor-argyll <none>

-- no debconf information

[Vincent Lefevre]

This maybe looks more like an issue in adduser?

Yes, I've just reassigned the bug to adduser.

So changing the shadow password field from '*' (no pwd) to '!'
(locked) allows adduser to pass.

Because

use constant {
EXISTING_NOT_FOUND => 0,
EXISTING_FOUND => 1,
EXISTING_SYSTEM => 2,
EXISTING_ID_MISMATCH => 4,
EXISTING_LOCKED => 8,
};

and the test is

if ($ret == (EXISTING_FOUND|EXISTING_SYSTEM)) {
# a user with this name already exists; it's a problem when it's not a system user
log_fatal( mtx("The user `%s' already exists, but is not a system user. Exiting."), $new_name );
exit( RET_WRONG_OBJECT_PROPERTIES );
}

So, if EXISTING_LOCKED is set, then the test will be false
and will not trigger the error.

BTW, I don't understand why the test is written to depend
on the lock status.

[Vincent Lefevre]

Control: severity 1099470 critical
Control: affects 1099477 colord
Control: merge 1099470 1099477

Correctly merging...

Note that the consequence of the incorrect test may even be worse:
It would not notice when the user is not a system user. This might
give unexpected rights to a normal user.

[Vincent Lefevre]

I found one way to workaround the issue is to uncomment part of
/etc/adduser.conf "FIRST_SYSTEM_UID" and "LAST_SYSTEM_UID", so the UID
check can be passed.

I don't see how this can solve the problem.

I believe that adduser should have their default values built in.

They are in the AdduserCommon.pm file:

first_system_uid => 100,
last_system_uid => 999,

[Marc Haber]

Control: retitle -1 fails upgrade of packages with locked system accounts
Control: tags -1 confirmed
thanks

I confirm this behavior in adduser 3.143. We are currently preparing
adduser for the post-trixie cycle. Those changes are not supposed to
change adduser's behavior, especially not in common cases.

The code in question was written to prepare adduser for addressing of
bugs #1008082 - #1008084. Again, a behavior changing change was only
planned for post-trixie. We are planning to add an option to lock an
account in postrm instead of deleting it. The contraire, unlocking the
account in postinst when adduser --system is called, is also planned.

It wasn't on my radar that many packages make use of an actually locked
account during the life of the package. I have made a note regarding
this.

I expect a fix for this adduser issue for later today and apologize for
the problem. There is no need to debug other packages until adduser
3.144 is uploaded.

Greetings
Marc

[Vincent Lefevre]

Control: retitle -1 fails upgrade of packages with non-locked system accounts

Post by Marc Haber
Control: retitle -1 fails upgrade of packages with locked system accounts

This is the opposite. The upgrade fails for system accounts that are
not locked (those without an exclamation point).

[Marc Haber]

Post by Vincent Lefevre
This is the opposite. The upgrade fails for system accounts that are
not locked (those without an exclamation point).

Just to get the terminology straight: How would you call an account that
has * as a password in /etc/shadow?

Greetings
Marc

[Marc Haber]

Post by Marc Haber
I expect a fix for this adduser issue for later today and apologize for
the problem. There is no need to debug other packages until adduser
3.144 is uploaded.

Can you try this trivial change?

diff --git a/adduser b/adduser
index 6fc7489..3166319 100755
--- a/adduser
+++ b/adduser
@@ -1154,6 +1154,7 @@ sub existing_user_status {
$ret |= EXISTING_SYSTEM if \
($uid >= $config{"first_system_uid"} && $uid <= $config{"last_system_uid"});
$ret |= EXISTING_LOCKED if (substr($pw,0,1) eq "!"); # TODO: also check expiry?
+ $ret |= EXISTING_LOCKED if (substr($pw,0,1) eq "*"); # TODO: also check expiry?
}
log_trace( "existing_user_status( %s, %s ) returns %s", $new_name, $new_uid, $ret );
return $ret;

I have, in a container, installed adduser and colord from testing,
changed /etc/shadow from colord:!:19977:::::: to colord:*:19977::::::,
upgraded adduser and colord. The bug issue was reproducible that way.
After patching adduser, the colord upgrade went through without issues.

And there is now a test case checking this.

I'll upload as soon as I get your feedback (or tonight).

Greetings
Marc

[Vincent Lefevre]

my $ret = existing_user_status($new_name, $new_uid);
if ($ret == (EXISTING_FOUND|EXISTING_SYSTEM)) {
# a user with this name already exists; it's a problem when it's not a system user
log_fatal( mtx("The user `%s' already exists, but is not a system user. Exiting."), $new_name );
exit( RET_WRONG_OBJECT_PROPERTIES );
}

I think that the test should just be

if (($ret & EXISTING_FOUND) && !($ret & EXISTING_SYSTEM)) {

But you could factorize the EXISTING_FOUND case as done for the
addsysgroup action:

my $ret = existing_user_status($new_name, $new_uid);
if ($ret == (EXISTING_FOUND|EXISTING_SYSTEM)) {
# a user with this name already exists; it's a problem when it's not a system user
log_fatal( mtx("The user `%s' already exists, but is not a system user. Exiting."), $new_name );
exit( RET_WRONG_OBJECT_PROPERTIES );
}
if ($ret & EXISTING_ID_MISMATCH) {
log_fatal( mtx("The user `%s' already exists with a different UID. Exiting."), $new_name );
exit( RET_WRONG_OBJECT_PROPERTIES );
}
if ($ret & EXISTING_FOUND) {
log_fatal( mtx("The system user `%s' already exists. Exiting.\n"), $new_name );
exit( RET_OK );
}

would be changed to

my $ret = existing_user_status($new_name, $new_uid);
if ($ret & EXISTING_FOUND) {
if (!($ret & EXISTING_SYSTEM)) {
# a user with this name already exists; it's a problem when it's not a system user
log_fatal( mtx("The user `%s' already exists, but is not a system user. Exiting."), $new_name );
exit( RET_WRONG_OBJECT_PROPERTIES );
}
if ($ret & EXISTING_ID_MISMATCH) {
log_fatal( mtx("The user `%s' already exists with a different UID. Exiting."), $new_name );
exit( RET_WRONG_OBJECT_PROPERTIES );
}
log_fatal( mtx("The system user `%s' already exists. Exiting.\n"), $new_name );
exit( RET_OK );
}

Not tested.