Merge ~ahasenack/ubuntu/+source/nfs-utils:jammy-nfs-utils-svcgssd-principal-1977745 into ubuntu/+source/nfs-utils:ubuntu/jammy-devel

Proposed by Andreas Hasenack
Status: Merged
Approved by: git-ubuntu bot
Approved revision: not available
Merged at revision: 45dc155a732074100137cf75bc0e56a7c880a7ba
Proposed branch: ~ahasenack/ubuntu/+source/nfs-utils:jammy-nfs-utils-svcgssd-principal-1977745
Merge into: ubuntu/+source/nfs-utils:ubuntu/jammy-devel
Diff against target: 203 lines (+163/-0)
6 files modified
debian/changelog (+14/-0)
debian/patches/nfs-conf-manpage-missing-svcgssd-options.patch (+19/-0)
debian/patches/series (+4/-0)
debian/patches/svcgssd-display-principal-if-set.patch (+37/-0)
debian/patches/svcgssd-document-missing-options.patch (+44/-0)
debian/patches/svcgssd-fix-use-after-free.patch (+45/-0)
Reviewer Review Type Date Requested Status
git-ubuntu bot Approve
Bryce Harrington (community) Approve
Canonical Server Reporter Pending
Review via email: mp+427771@code.launchpad.net

Description of the change

Bringing in one set of fixes from kinetic to jammy. I was planning on bundling these with other fixes we have in kinetic, but I didn't get feedback yet on those, so I'll leave them cooking in kinetic for a while longer and proceed with this SRU, which is more straighforward.

The linked bug has the necessary test cases.

PPA: https://launchpad.net/~ahasenack/+archive/ubuntu/nfs-utils-svcgssd-principal-1977745/

I just kicked the DEP8 tests, will post back in a while after there are results.

To post a comment you must log in.
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Putting this back to "work in progress" because, after talking to the security team, I'll take this update opportunity and also fix https://bugs.launchpad.net/ubuntu/+source/nfs-utils/+bug/1980095

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Back in business with the extra hardening fix.

Revision history for this message
Bryce Harrington (bryce) wrote :

The DEP8 tests from yesterday all passed. I've re-triggered to pick up today's changes but think maybe the ppa hasn't been updated?

Results: (from http://autopkgtest.ubuntu.com/results/autopkgtest-jammy-ahasenack-nfs-utils-svcgssd-principal-1977745/?format=plain)
  nfs-utils @ amd64:
    03.08.22 18:31:23 Log 🗒️ ✅ Triggers: nfs-utils/1:2.6.1-1ubuntu1.1~ppa2
  nfs-utils @ arm64:
    03.08.22 18:36:34 Log 🗒️ ✅ Triggers: nfs-utils/1:2.6.1-1ubuntu1.1~ppa2
  nfs-utils @ armhf:
    03.08.22 18:21:08 Log 🗒️ ✅ Triggers: nfs-utils/1:2.6.1-1ubuntu1.1~ppa2
  nfs-utils @ ppc64el:
    03.08.22 18:36:01 Log 🗒️ ✅ Triggers: nfs-utils/1:2.6.1-1ubuntu1.1~ppa2
  nfs-utils @ s390x:
    03.08.22 18:27:38 Log 🗒️ ✅ Triggers: nfs-utils/1:2.6.1-1ubuntu1.1~ppa2
Running: (none)
Waiting: (none)

In any case changes all LGTM. Verified they match what landed in Kinetic.

Also reviewed the SRU text for both bugs, I didn't run through the test cases but they look very thorough and well documented.

review: Approve
Revision history for this message
git-ubuntu bot (git-ubuntu-bot) wrote :

Approvers: ahasenack, bryce
Uploaders: ahasenack, bryce
MP auto-approved

review: Approve
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

I see the ppa2 version of the package in the DEP8 logs, so it seems fine now, and the date is from today:

Results: (from http://autopkgtest.ubuntu.com/results/autopkgtest-jammy-ahasenack-nfs-utils-svcgssd-principal-1977745/?format=plain)
  nfs-utils @ amd64:
    04.08.22 03:28:56 Log 🗒️ ✅ Triggers: nfs-utils/1:2.6.1-1ubuntu1.1~ppa2
  nfs-utils @ arm64:
    04.08.22 03:28:23 Log 🗒️ ✅ Triggers: nfs-utils/1:2.6.1-1ubuntu1.1~ppa2
  nfs-utils @ armhf:
    04.08.22 03:10:38 Log 🗒️ ✅ Triggers: nfs-utils/1:2.6.1-1ubuntu1.1~ppa2
  nfs-utils @ ppc64el:
    04.08.22 03:21:24 Log 🗒️ ✅ Triggers: nfs-utils/1:2.6.1-1ubuntu1.1~ppa2
  nfs-utils @ s390x:
    04.08.22 03:16:47 Log 🗒️ ✅ Triggers: nfs-utils/1:2.6.1-1ubuntu1.1~ppa2
Running: (none)
Waiting: (none)

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Uploaded, now it's up to the SRU team:

Uploading nfs-utils_2.6.1-1ubuntu1.1.dsc
Uploading nfs-utils_2.6.1-1ubuntu1.1.debian.tar.xz
Uploading nfs-utils_2.6.1-1ubuntu1.1_source.buildinfo
Uploading nfs-utils_2.6.1-1ubuntu1.1_source.changes

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/debian/changelog b/debian/changelog
index c8ab091..92e7b2e 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,17 @@
1nfs-utils (1:2.6.1-1ubuntu1.1) jammy; urgency=medium
2
3 * rpc.svcgssd fixes and improvements (LP: #1977745):
4 - d/p/svcgssd-fix-use-after-free.patch: fix use-after-free which was
5 preventing svcgssd options set in /etc/nfs.conf from being used
6 - d/p/svcgssd-display-principal-if-set.patch: improve logging,
7 showing the expected principal name if it was set in the config
8 - d/p/svcgssd-document-missing-options.patch: add missing options to
9 the svcgssd manpage
10 - d/p/nfs-conf-manpage-missing-svcgssd-options.patch: also
11 document the missing svcgssd options to the nfs.conf(5) manpage
12
13 -- Andreas Hasenack <andreas@canonical.com> Wed, 14 Sep 2022 14:34:00 -0300
14
1nfs-utils (1:2.6.1-1ubuntu1) jammy; urgency=medium15nfs-utils (1:2.6.1-1ubuntu1) jammy; urgency=medium
216
3 * Merge with Debian unstable (LP: #1960829). Remaining changes:17 * Merge with Debian unstable (LP: #1960829). Remaining changes:
diff --git a/debian/patches/nfs-conf-manpage-missing-svcgssd-options.patch b/debian/patches/nfs-conf-manpage-missing-svcgssd-options.patch
4new file mode 10064418new file mode 100644
index 0000000..b57ad8c
--- /dev/null
+++ b/debian/patches/nfs-conf-manpage-missing-svcgssd-options.patch
@@ -0,0 +1,19 @@
1Description: add missing svcgssd long options to nfs.conf(5)
2Author: Andreas Hasenack <andreas@canonical.com>
3Forwarded: https://marc.info/?l=linux-nfs&m=165635622607689&w=4
4Last-Update: 2022-06-27
5
6--- a/systemd/nfs.conf.man
7+++ b/systemd/nfs.conf.man
8@@ -283,7 +283,10 @@
9 .TP
10 .B svcgssd
11 Recognized values:
12-.BR principal .
13+.BR principal ,
14+.BR verbosity ,
15+.BR rpc-verbosity ,
16+.BR idmap-verbosity .
17
18 See
19 .BR rpc.svcgssd (8)
diff --git a/debian/patches/series b/debian/patches/series
index 127f3e1..5626161 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -4,3 +4,7 @@ multiarch-kerberos-paths.patch
4nfs-utils-fix-man-page-syntax-errors.patch4nfs-utils-fix-man-page-syntax-errors.patch
5tests-skip-test-if-dev-log-is-missing.patch5tests-skip-test-if-dev-log-is-missing.patch
6remove-regex-from-docs.patch6remove-regex-from-docs.patch
7svcgssd-fix-use-after-free.patch
8svcgssd-display-principal-if-set.patch
9svcgssd-document-missing-options.patch
10nfs-conf-manpage-missing-svcgssd-options.patch
diff --git a/debian/patches/svcgssd-display-principal-if-set.patch b/debian/patches/svcgssd-display-principal-if-set.patch
7new file mode 10064411new file mode 100644
index 0000000..a10edd8
--- /dev/null
+++ b/debian/patches/svcgssd-display-principal-if-set.patch
@@ -0,0 +1,37 @@
1From 284d249e0fe58443dafc96fa8be51a2cef4541a0 Mon Sep 17 00:00:00 2001
2From: Marcel Ritter <marcel@linux-ng.de>
3Date: Tue, 21 Jun 2022 09:21:36 -0400
4Subject: [PATCH] svcgssd: Display principal if set
5
6It's a little irritating to only see the template "<...>@<...>" if you
7set a specific principal name. So let's show it (if set).
8
9Signed-off-by: Steve Dickson <steved@redhat.com>
10---
11 utils/gssd/svcgssd.c | 6 +++---
12 1 file changed, 3 insertions(+), 3 deletions(-)
13
14Origin: upstream, http://git.linux-nfs.org/?p=steved/nfs-utils.git;a=commit;h=284d249e0fe58443dafc96fa8be51a2cef4541a0
15Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/nfs-utils/+bug/1977745
16Last-Update: 2022-06-27
17
18diff --git a/utils/gssd/svcgssd.c b/utils/gssd/svcgssd.c
19index a242b78..ce78d8f 100644
20--- a/utils/gssd/svcgssd.c
21+++ b/utils/gssd/svcgssd.c
22@@ -295,9 +295,9 @@ main(int argc, char *argv[])
23 (const gss_OID)GSS_C_NT_HOSTBASED_SERVICE);
24 if (status == FALSE) {
25 printerr(0, "unable to obtain root (machine) credentials\n");
26- printerr(0, "do you have a keytab entry for "
27- "nfs/<your.host>@<YOUR.REALM> in "
28- "/etc/krb5.keytab?\n");
29+ printerr(0, "do you have a keytab entry for %s in"
30+ "/etc/krb5.keytab?\n",
31+ principal ? principal : "nfs/<your.host>@<YOUR.REALM>");
32 exit(1);
33 }
34 } else {
35--
361.8.3.1
37
diff --git a/debian/patches/svcgssd-document-missing-options.patch b/debian/patches/svcgssd-document-missing-options.patch
0new file mode 10064438new file mode 100644
index 0000000..18cf721
--- /dev/null
+++ b/debian/patches/svcgssd-document-missing-options.patch
@@ -0,0 +1,44 @@
1From f541550358f136e9a6d1fd131e83d17e6269dae4 Mon Sep 17 00:00:00 2001
2From: Marcel Ritter <marcel@linux-ng.de>
3Date: Tue, 21 Jun 2022 09:23:22 -0400
4Subject: [PATCH] svcgssd: Add (undocumented) config options to man page
5
6There seem to be some undocumented options implemented.
7Why not mention them in the man page?
8
9Signed-off-by: Steve Dickson <steved@redhat.com>
10---
11 utils/gssd/svcgssd.man | 13 +++++++++++++
12 1 file changed, 13 insertions(+)
13
14Origin: upstream, http://git.linux-nfs.org/?p=steved/nfs-utils.git;a=commit;h=f541550358f136e9a6d1fd131e83d17e6269dae4
15Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/nfs-utils/+bug/1977745
16Last-Update: 2022-06-27
17
18diff --git a/utils/gssd/svcgssd.man b/utils/gssd/svcgssd.man
19index 15ef4c9..8771c03 100644
20--- a/utils/gssd/svcgssd.man
21+++ b/utils/gssd/svcgssd.man
22@@ -61,6 +61,19 @@ this is equivalent to the
23 option. If set to any other value, that is used like the
24 .B -p
25 option.
26+.TP
27+.B verbosity
28+Value which is equivalent to the number of
29+.BR -v .
30+.TP
31+.B rpc-verbosity
32+Value which is equivalent to the number of
33+.BR -r .
34+.TP
35+.B idmap-verbosity
36+Value which is equivalent to the number of
37+.BR -i .
38+
39
40 .SH SEE ALSO
41 .BR rpc.gssd(8),
42--
431.8.3.1
44
diff --git a/debian/patches/svcgssd-fix-use-after-free.patch b/debian/patches/svcgssd-fix-use-after-free.patch
0new file mode 10064445new file mode 100644
index 0000000..5a9b0a6
--- /dev/null
+++ b/debian/patches/svcgssd-fix-use-after-free.patch
@@ -0,0 +1,45 @@
1From 2eabb25d5a43e48e769a0db29956e9f5dc5b5913 Mon Sep 17 00:00:00 2001
2From: Marcel Ritter <marcel@linux-ng.de>
3Date: Tue, 21 Jun 2022 09:19:17 -0400
4Subject: [PATCH] svcgssd: Fix use-after-free bug (config variables)
5
6This patch fixes a bug when trying to set "principal" in /etc/nfs.conf.
7Memory gets freed by conf_cleanup() before being used - moving cleanup
8code resolves that.
9
10Signed-off-by: Steve Dickson <steved@redhat.com>
11---
12 utils/gssd/svcgssd.c | 6 +++---
13 1 file changed, 3 insertions(+), 3 deletions(-)
14
15Origin: upstream, http://git.linux-nfs.org/?p=steved/nfs-utils.git;a=commit;h=2eabb25d5a43e48e769a0db29956e9f5dc5b5913
16Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/nfs-utils/+bug/1977745
17Last-Update: 2022-06-27
18
19diff --git a/utils/gssd/svcgssd.c b/utils/gssd/svcgssd.c
20index 881207b..a242b78 100644
21--- a/utils/gssd/svcgssd.c
22+++ b/utils/gssd/svcgssd.c
23@@ -211,9 +211,6 @@ main(int argc, char *argv[])
24 rpc_verbosity = conf_get_num("svcgssd", "RPC-Verbosity", rpc_verbosity);
25 idmap_verbosity = conf_get_num("svcgssd", "IDMAP-Verbosity", idmap_verbosity);
26
27- /* We don't need the config anymore */
28- conf_cleanup();
29-
30 while ((opt = getopt(argc, argv, "fivrnp:")) != -1) {
31 switch (opt) {
32 case 'f':
33@@ -328,6 +325,9 @@ main(int argc, char *argv[])
34
35 daemon_ready();
36
37+ /* We don't need the config anymore */
38+ conf_cleanup();
39+
40 nfs4_init_name_mapping(NULL); /* XXX: should only do this once */
41
42 rc = event_base_dispatch(evbase);
43--
441.8.3.1
45

Subscribers

People subscribed via source and target branches