[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1031725: marked as done (unblock: accountsservice/22.08.8-6)

Your message dated Tue, 21 Feb 2023 18:02:21 +0200
with message-id <CAM8zJQs2mih2p=6S-TNfffs5c=wEQHCLsWwn+bdi95KpSPXm2A@mail.gmail.com>
and subject line Re: Bug#1031725: unblock: accountsservice/22.08.8-6
has caused the Debian Bug report #1031725,
regarding unblock: accountsservice/22.08.8-6
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
1031725: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1031725
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message --- 
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock
X-Debbugs-Cc: accountsservice@packages.debian.org
Control: affects -1 + src:accountsservice

accountsservice could benefit from some appropriate age-days to get it
into testing a bit sooner. It's currently at 3 of 10 days.

(Do you want requests like this for important/RC bug fixes during soft
freeze in general, or should maintainers prefer to wait 10 days and hope
that manual intervention isn't needed?)

[ Reason ]
Fixes a regression introduced while fixing previous RC bugs.

[ Impact ]
If not accepted, accounts-daemon crashes with an assertion failure on
systems where /etc/shadow doesn't exist (pwunconv or shadowconfig off),
which is inadvisable but at least partially supported. This leads to
gdm and probably other login managers not starting, preventing login.
(#1031309)

This is a regression caused by my previous fix for #1030262, which is
already in testing. At the time of fixing #1030262 it hadn't occurred
to me that a missing /etc/shadow was something that could happen on a
supportable system.

[ Tests ]
The scenario with the crash was manually tested in a bookworm GNOME VM. It
seemed too rare and weird to add an automated test, but the automated test
added while fixing #1030262 and #1030253 still passes in this version.

[ Risks ]
The service is high-visibility and security-sensitive, but the change
is straightforward (allowing a hash table to be NULL) and was already
accepted upstream.

[ Checklist ]
  [x] all changes are documented in the d/changelog
  [x] I reviewed all changes and I approve them
  [x] attach debdiff against the package in testing

unblock accountsservice/22.08.8-6

diffstat for accountsservice-22.08.8 accountsservice-22.08.8

 debian/changelog                                                               |   10 ++
 debian/patches/0005-gdm_config_file_path.patch                                 |    7 -
 debian/patches/Annotate-varargs-functions-with-G_GNUC_PRINTF.patch             |    1 
 debian/patches/daemon-Don-t-crash-if-etc-shadow-doesn-t-exist.patch            |   41 ++++++++++
 debian/patches/series                                                          |    1 
 debian/patches/tests-fix-invocation-of-AddUser.patch                           |    2 
 debian/patches/tests-fix-the-signature-for-the-SetLocked-call.patch            |    2 
 debian/patches/user-Use-correct-format-strings-to-print-accounts_user_ge.patch |    1 
 src/daemon.c                                                                   |    5 -
 9 files changed, 61 insertions(+), 9 deletions(-)

diff -Nru accountsservice-22.08.8/debian/changelog accountsservice-22.08.8/debian/changelog
--- accountsservice-22.08.8/debian/changelog	2023-02-07 13:46:10.000000000 +0000
+++ accountsservice-22.08.8/debian/changelog	2023-02-18 09:22:31.000000000 +0000
@@ -1,3 +1,13 @@
+accountsservice (22.08.8-6) unstable; urgency=medium
+
+  * Team upload
+  * d/p/daemon-Don-t-crash-if-etc-shadow-doesn-t-exist.patch:
+    Add patch to prevent a crash if /etc/shadow is missing or empty
+    (Closes: #1031309)
+  * d/patches: Mark most patches as having been applied upstream
+
+ -- Simon McVittie <smcv@debian.org>  Sat, 18 Feb 2023 09:22:31 +0000
+
 accountsservice (22.08.8-5) unstable; urgency=medium
 
   * Team upload
diff -Nru accountsservice-22.08.8/debian/patches/0005-gdm_config_file_path.patch accountsservice-22.08.8/debian/patches/0005-gdm_config_file_path.patch
--- accountsservice-22.08.8/debian/patches/0005-gdm_config_file_path.patch	2023-02-07 13:46:10.000000000 +0000
+++ accountsservice-22.08.8/debian/patches/0005-gdm_config_file_path.patch	2023-02-18 09:22:31.000000000 +0000
@@ -1,11 +1,10 @@
 From: Josselin Mouette <joss@debian.org>
 Date: Sat, 12 Oct 2019 10:29:08 +0200
-Subject: Fix path to the GDM configuration file, which is different
+Subject: Fix path to the GDM configuration file
 
-Bug-Debian: http://bugs.debian.org/627311
-Bug: https://bugs.freedesktop.org/show_bug.cgi?id=49993
+It's different in Debian.
 
-in Debian.
+Bug-Debian: http://bugs.debian.org/627311
 Bug: https://bugs.freedesktop.org/show_bug.cgi?id=49993
 ---
  src/daemon.c | 2 +-
diff -Nru accountsservice-22.08.8/debian/patches/Annotate-varargs-functions-with-G_GNUC_PRINTF.patch accountsservice-22.08.8/debian/patches/Annotate-varargs-functions-with-G_GNUC_PRINTF.patch
--- accountsservice-22.08.8/debian/patches/Annotate-varargs-functions-with-G_GNUC_PRINTF.patch	2023-02-07 13:46:10.000000000 +0000
+++ accountsservice-22.08.8/debian/patches/Annotate-varargs-functions-with-G_GNUC_PRINTF.patch	2023-02-18 09:22:31.000000000 +0000
@@ -8,6 +8,7 @@
 Bug: https://gitlab.freedesktop.org/accountsservice/accountsservice/-/issues/109
 Signed-off-by: Simon McVittie <smcv@debian.org>
 Forwarded: https://gitlab.freedesktop.org/accountsservice/accountsservice/-/merge_requests/120
+Applied-upstream: 23.0, commit:c142812f7653cd1f6e52224da8410cd09f102a4f
 ---
  src/daemon.c | 5 +++++
  src/user.c   | 5 +++++
diff -Nru accountsservice-22.08.8/debian/patches/daemon-Don-t-crash-if-etc-shadow-doesn-t-exist.patch accountsservice-22.08.8/debian/patches/daemon-Don-t-crash-if-etc-shadow-doesn-t-exist.patch
--- accountsservice-22.08.8/debian/patches/daemon-Don-t-crash-if-etc-shadow-doesn-t-exist.patch	1970-01-01 01:00:00.000000000 +0100
+++ accountsservice-22.08.8/debian/patches/daemon-Don-t-crash-if-etc-shadow-doesn-t-exist.patch	2023-02-18 09:22:31.000000000 +0000
@@ -0,0 +1,41 @@
+From: Simon McVittie <smcv@debian.org>
+Date: Fri, 17 Feb 2023 23:59:12 +0000
+Subject: daemon: Don't crash if /etc/shadow doesn't exist
+
+Turning off shadow passwords with `shadowconfig off` or `pwunconv` is
+something that distributions still at least half-support, and
+apparently some people genuinely do this. In this situation, treat all
+users as non-local (until cached) but don't crash.
+
+Bug-Debian: https://bugs.debian.org/1031309
+Signed-off-by: Simon McVittie <smcv@debian.org>
+Forwarded: https://gitlab.freedesktop.org/accountsservice/accountsservice/-/merge_requests/121
+Applied-upstream: 23.0, commit:322165ea2e1c1c4715d532910ccb31b3d1e0a04e
+---
+ src/daemon.c | 5 ++---
+ 1 file changed, 2 insertions(+), 3 deletions(-)
+
+diff --git a/src/daemon.c b/src/daemon.c
+index c29a8cf..8fd91fe 100644
+--- a/src/daemon.c
++++ b/src/daemon.c
+@@ -494,7 +494,6 @@ reload_users (Daemon *daemon)
+ 
+         /* Load the local users into our hash tables */
+         load_entries (daemon, users, FALSE, entry_generator_fgetpwent, &local);
+-        g_assert (local != NULL);
+ 
+         /* Now add/update users from other sources, possibly non-local */
+         load_entries (daemon, users, TRUE, entry_generator_cachedir, NULL);
+@@ -510,9 +509,9 @@ reload_users (Daemon *daemon)
+                 User *user = value;
+                 if (!user_get_system_account (user))
+                         number_of_normal_users++;
+-                user_update_local_account_property (user, g_hash_table_lookup (local, name) != NULL);
++                user_update_local_account_property (user, local != NULL && g_hash_table_lookup (local, name) != NULL);
+         }
+-        g_hash_table_destroy (local);
++        g_clear_pointer (&local, g_hash_table_destroy);
+ 
+         had_no_users = accounts_accounts_get_has_no_users (accounts);
+         has_no_users = number_of_normal_users == 0;
diff -Nru accountsservice-22.08.8/debian/patches/series accountsservice-22.08.8/debian/patches/series
--- accountsservice-22.08.8/debian/patches/series	2023-02-07 13:46:10.000000000 +0000
+++ accountsservice-22.08.8/debian/patches/series	2023-02-18 09:22:31.000000000 +0000
@@ -8,3 +8,4 @@
 daemon-Define-local-users-as-being-exactly-those-present-.patch
 user-Use-correct-format-strings-to-print-accounts_user_ge.patch
 Annotate-varargs-functions-with-G_GNUC_PRINTF.patch
+daemon-Don-t-crash-if-etc-shadow-doesn-t-exist.patch
diff -Nru accountsservice-22.08.8/debian/patches/tests-fix-invocation-of-AddUser.patch accountsservice-22.08.8/debian/patches/tests-fix-invocation-of-AddUser.patch
--- accountsservice-22.08.8/debian/patches/tests-fix-invocation-of-AddUser.patch	2023-02-07 13:46:10.000000000 +0000
+++ accountsservice-22.08.8/debian/patches/tests-fix-invocation-of-AddUser.patch	2023-02-18 09:22:31.000000000 +0000
@@ -7,7 +7,7 @@
 before dbusmock commit f8709a9 because these methods were never really
 looked at.
 
-Origin: https://gitlab.freedesktop.org/accountsservice/accountsservice/-/commit/c588aea0
+Origin: upstream, 23.0, commit:https://gitlab.freedesktop.org/accountsservice/accountsservice/-/commit/c588aea0
 ---
  tests/dbusmock/accounts_service.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
diff -Nru accountsservice-22.08.8/debian/patches/tests-fix-the-signature-for-the-SetLocked-call.patch accountsservice-22.08.8/debian/patches/tests-fix-the-signature-for-the-SetLocked-call.patch
--- accountsservice-22.08.8/debian/patches/tests-fix-the-signature-for-the-SetLocked-call.patch	2023-02-07 13:46:10.000000000 +0000
+++ accountsservice-22.08.8/debian/patches/tests-fix-the-signature-for-the-SetLocked-call.patch	2023-02-18 09:22:31.000000000 +0000
@@ -4,7 +4,7 @@
 
 It's a boolean, not a string
 
-Origin: https://gitlab.freedesktop.org/accountsservice/accountsservice/-/commit/fea3ecdc
+Origin: upstream, 23.0, commit:https://gitlab.freedesktop.org/accountsservice/accountsservice/-/commit/fea3ecdc
 ---
  tests/dbusmock/accounts_service.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
diff -Nru accountsservice-22.08.8/debian/patches/user-Use-correct-format-strings-to-print-accounts_user_ge.patch accountsservice-22.08.8/debian/patches/user-Use-correct-format-strings-to-print-accounts_user_ge.patch
--- accountsservice-22.08.8/debian/patches/user-Use-correct-format-strings-to-print-accounts_user_ge.patch	2023-02-07 13:46:10.000000000 +0000
+++ accountsservice-22.08.8/debian/patches/user-Use-correct-format-strings-to-print-accounts_user_ge.patch	2023-02-18 09:22:31.000000000 +0000
@@ -15,6 +15,7 @@
 Bug: https://gitlab.freedesktop.org/accountsservice/accountsservice/-/issues/109
 Signed-off-by: Simon McVittie <smcv@debian.org>
 Forwarded: https://gitlab.freedesktop.org/accountsservice/accountsservice/-/merge_requests/120
+Applied-upstream: 23.0, commit:a1a330b8720e4bc1c2154f120196372627dc7b2a
 ---
  src/user.c | 24 ++++++++++++------------
  1 file changed, 12 insertions(+), 12 deletions(-)
diff -Nru accountsservice-22.08.8/src/daemon.c accountsservice-22.08.8/src/daemon.c
--- accountsservice-22.08.8/src/daemon.c	2023-02-21 14:46:04.000000000 +0000
+++ accountsservice-22.08.8/src/daemon.c	2023-02-21 14:46:04.000000000 +0000
@@ -494,7 +494,6 @@
 
         /* Load the local users into our hash tables */
         load_entries (daemon, users, FALSE, entry_generator_fgetpwent, &local);
-        g_assert (local != NULL);
 
         /* Now add/update users from other sources, possibly non-local */
         load_entries (daemon, users, TRUE, entry_generator_cachedir, NULL);
@@ -510,9 +509,9 @@
                 User *user = value;
                 if (!user_get_system_account (user))
                         number_of_normal_users++;
-                user_update_local_account_property (user, g_hash_table_lookup (local, name) != NULL);
+                user_update_local_account_property (user, local != NULL && g_hash_table_lookup (local, name) != NULL);
         }
-        g_hash_table_destroy (local);
+        g_clear_pointer (&local, g_hash_table_destroy);
 
         had_no_users = accounts_accounts_get_has_no_users (accounts);
         has_no_users = number_of_normal_users == 0;

--- End Message ---
--- Begin Message --- 
Hi Simon

On Tue, 21 Feb 2023 at 17:09, Simon McVittie <smcv@debian.org> wrote:
> accountsservice could benefit from some appropriate age-days to get it
> into testing a bit sooner. It's currently at 3 of 10 days.

I've added a hint, accountsservice should migrate soon.

> (Do you want requests like this for important/RC bug fixes during soft
> freeze in general, or should maintainers prefer to wait 10 days and hope
> that manual intervention isn't needed?)

If a maintainer feels getting a fix into testing is urgent enough,
they can request an unblock.

Regards
Graham

--- End Message ---
Reply to: