Merge ~paride/ubuntu/+source/samba:lp1942195-impish into ubuntu/+source/samba:ubuntu/impish-devel

[Paride Legovini]

Impish SRU for LP: #1942195, see the SRU template for more info testing instructions.
Test PPA: Test PPA: https://launchpad.net/~paride/+archive/ubuntu/samba-lp1942195

Autopkgtest:

@@@@@@@@@@@@@@@@@@@@ summary
cifs-share-access SKIP Test requires machine-level isolation but testbed does not provide that
cifs-share-access-uring SKIP Test requires machine-level isolation but testbed does not provide that
python-smoke PASS
smbclient-anonymous-share-list PASS
smbclient-authenticated-share-list PASS
smbclient-share-access PASS
smbclient-share-access-uring PASS

NOTE: this was already uploaded to Impish, but the upload has been removed from Impish and released to Jammy, see:

  https://launchpad.net/ubuntu/+source/samba/2:4.13.5+dfsg-2ubuntu3

This happened because it was uploaded to little before the Impish final freeze. For this reason version 2:4.13.5+dfsg-2ubuntu3 appears in the ubuntu/impish-devel importer branch, even if it's not in the archive (because of LP: #1852389). The proposed change change simply adjusts the version string as we're re-uploading the package to Impish, now as a SRU.

There's a corner case where the 2:4.13.5+dfsg-2ubuntu2.1 version string could be wrong: if a system installed samba 2:4.13.5+dfsg-2ubuntu3 from impish-proposed *before* it got deleted, then the new version (and the following SRUs) will have a lower version than the one of the installed package, and won't get installed. If we want to take into account this possibility then the version string for the new upload should be something like:

  2:4.13.5+dfsg-2ubuntu3~ubuntu21.10.1

What do you think?

[Sergio Durigan Junior (sergiodj)]

Thanks for the MP and the explanation, Paride.

IIUC the problem correctly, we want to make sure that:

1) The new version (which we will upload to Impish because of this MP) is higher than the existing version currently in Impish (2:4.13.5+dfsg-2ubuntu2).

2) The new version is not higher than 2:4.13.5+dfsg-2ubuntu3, which is released in Jammy and could be part of an Impish system if the user has installed samba 2:4.13.5+dfsg-2ubuntu3 before it got deleted from impish-proposed.

The version you suggested above will satisfy both conditions, but I'm afraid that if someone decides to do another SRU for samba in Impish, this person will mistakenly bump the "~ubuntu21.10.1" part of the version string, which will basically render that SRU useless because it will not be considered as an upgrade:

# This is assuming a new SRU versioned with the suffix ~ubuntu21.10.2
$ dpkg --compare-versions '2:4.13.5+dfsg-2ubuntu3' lt '2:4.13.5+dfsg-2ubuntu3~ubuntu21.10.2'; echo $?
1

I think we can either:

a) Bite the bullet, perform a no-change upload of samba to Jammy just to bump its version to -2ubuntu4, and then use -ubuntu3.1 (or -ubuntu3.20.10.1) as the Impish version, which will have the downside of forcing users who already got the fix (because they installed samba using impish-proposed and before the package got deleted) to install it again. IMO this is not so bad: I'm not expecting that a lot of people will have this fixed samba installed, and if they do they chose to install it from -proposed which is always risky.

b) Leave a comment or some kind of breadcrumb on the Impish's debian/changelog entry warning the next uploader that she will have to remove the "~ubuntu20.10.1" suffix from the version (and bump it to -2ubuntu3.21.10.1, given that -2ubuntu3 has already been taken and -2ubuntu4 is reserved for Jammy) when there's a next SRU/security upload.

IMHO, I prefer (a) because it sounds like the most robust option. (b) sounds like a fragile solution and I can totally see a situation where the next uploader won't bother reading the previous changelog entry and make a mistake here (and the worst part is that there won't be any alarms going off if this mistake happens).

I won't be around Thursday, but feel free to think about these scenarios and let me know what you prefer. Also, if other team members would like to chip in, feel free to do so.

[Paride Legovini (paride)]

Thanks for your well thought comment Sergio. I also prefer option (a), I see no good reason for avoiding a no-change upload to Jammy (yes we force a useless update, but it's the devel release after all). The outcome will be the most robust one and we'll not have to remember about this specific case in the future Impish SRUs.

[Paride Legovini (paride)]

I filed LP: #1950626 to serve as a pointer to explain/justify the no-change upload.

[Paride Legovini (paride)]

I adjusted the version string to implement (a), but first we need get this one

https://code.launchpad.net/~paride/ubuntu/+source/samba/+git/samba/+merge/411741

land to Jammy. I'm leaving this MR marked as WIP to signal that.

Full autopkgtest results:

autopkgtest [12:31:04]: @@@@@@@@@@@@@@@@@@@@ summary
cifs-share-access PASS
cifs-share-access-uring PASS
python-smoke PASS
smbclient-anonymous-share-list PASS
smbclient-authenticated-share-list PASS
smbclient-share-access PASS
smbclient-share-access-uring PASS

[Paride Legovini (paride)]

This Impish security upload:

https://launchpad.net/ubuntu/+source/samba/2:4.13.14+dfsg-0ubuntu0.21.10.1

resolved the issue by bumping the upstream part of the version string. No need to proceed with the Jammy no-change upload anymore. In any case I'm happy that now I know to handle this specific corner case.

I'll have to change the version string once again (rebasing on that security upload), but the importer doesn't have it yet. I'll check again later today or tomorrow. The MP remains In Progress.

[Paride Legovini (paride)]

@Sergio rebased on 2:4.13.14+dfsg-0ubuntu0.21.10.1, up for review again.

[Paride Legovini (paride)]

New autopkgtest results:

autopkgtest [16:27:56]: @@@@@@@@@@@@@@@@@@@@ summary
cifs-share-access PASS
cifs-share-access-uring PASS
python-smoke PASS
smbclient-anonymous-share-list PASS
smbclient-authenticated-share-list PASS
smbclient-share-access PASS
smbclient-share-access-uring PASS

[Sergio Durigan Junior (sergiodj)]

Thanks for looking into bumping the version on Jammy, and then for sync'ing with the security team! Indeed, this was an interesting problem to think about :-).

This LGTM now.

Uploaded:

$ dput samba_4.13.14+dfsg-0ubuntu0.21.10.2_source.changes
Trying to upload package to ubuntu
Checking signature on .changes
gpg: /home/sergio/work/samba/samba_4.13.14+dfsg-0ubuntu0.21.10.2_source.changes: Valid signature from 106DA1C8C3CBBF14
Checking signature on .dsc
gpg: /home/sergio/work/samba/samba_4.13.14+dfsg-0ubuntu0.21.10.2.dsc: Valid signature from 106DA1C8C3CBBF14
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading samba_4.13.14+dfsg-0ubuntu0.21.10.2.dsc: done.
  Uploading samba_4.13.14+dfsg-0ubuntu0.21.10.2.debian.tar.xz: done.
  Uploading samba_4.13.14+dfsg-0ubuntu0.21.10.2_source.buildinfo: done.
  Uploading samba_4.13.14+dfsg-0ubuntu0.21.10.2_source.changes: done.
Successfully uploaded packages.

review: Approve

diff --git a/debian/changelog b/debian/changelog
index ac64100..8de0b91 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+samba (2:4.13.14+dfsg-0ubuntu0.21.10.2) impish; urgency=medium
+
+  * samba.postinst: do not populate sambashare from the Ubuntu admin group
+    (LP: #1942195)
+
+ -- Paride Legovini <[e-mail]>  Fri, 12 Nov 2021 11:17:14 +0100
+
 samba (2:4.13.14+dfsg-0ubuntu0.21.10.1) impish-security; urgency=medium
 
   * Update to 4.13.14 as a security update (LP: #1950363)
diff --git a/debian/samba.postinst b/debian/samba.postinst
index 79e688a..eb327a2 100644
--- a/debian/samba.postinst
+++ b/debian/samba.postinst
@@ -59,20 +59,6 @@ umask 022
 if ! getent group sambashare > /dev/null 2>&1
 then
 	addgroup --system sambashare
-	# Only on Ubuntu, use the "admin" group as a template for the
-	# initial users for this group; Debian has no equivalent group,
-	# so leaving the sambashare group empty is the more secure default
-	if [ -x "`which lsb_release 2>/dev/null`" ] \
-	   && [ "`lsb_release -s -i`" = "Ubuntu" ]
-	then
-		OLDIFS="$IFS"
-		IFS=","
-		for USER in `getent group admin | cut -f4 -d:`; do
-			adduser "$USER" sambashare \
-			|| ! getent passwd "$USER" >/dev/null
-		done
-		IFS="$OLDIFS"
-	fi
 fi
 
 if [ ! -e /var/lib/samba/usershares ]