Merge ~ahasenack/ubuntu/+source/libnfsidmap:bionic-uid-map-krb5-1819197 into ubuntu/+source/libnfsidmap:ubuntu/bionic-devel

Proposed by Andreas Hasenack
Status: Work in progress
Proposed branch: ~ahasenack/ubuntu/+source/libnfsidmap:bionic-uid-map-krb5-1819197
Merge into: ubuntu/+source/libnfsidmap:ubuntu/bionic-devel
Diff against target: 59 lines (+38/-0)
3 files modified
debian/changelog (+7/-0)
debian/patches/03-uid-map-krb5.patch (+30/-0)
debian/patches/series (+1/-0)
Reviewer Review Type Date Requested Status
Canonical Server Core Reviewers Pending
Christian Ehrhardt  Pending
Review via email: mp+364953@code.launchpad.net

This proposal supersedes a proposal from 2019-03-21.

Description of the change

This fixes a very old bug, reported multiple times in both debian[1] and ubuntu[2].

Testing it might be a bit complicated, I just so happened to have a kerberos + nfsv4 setup at home already, I'll think of an easy way to set something up when the SRU time comes.

There have been two versions of the fix floating around across all these bugs. Compounded with that, there doesn't seem to be an official upstream anymore.

Patch 1[3], which is what I used here, fixes nss.c:strip_domain() to look for the last "@" sign in the username, and strip that off, instead of the first. This is what fixes the case when a username already has an "@" sign, which is the default in sssd. A package from my PPA[4] with this patch was tested by the bug reporter, and confirmed working.

The second patch[5] has that hunk, plus another one to fix nss.c:nss_gss_princ_to_ids() in a similar manner:

- princ_realm = strstr(princ, "@");
+ princ_realm = strrchr(princ, '@');

It's not clear to me when that function is used. I would have to understand that to check what kind of (incorrect) values could be given to the "princ" variable. If it's really just given a principal (which is of the form user@REALM), then both strstr() and strrchr() would yield the same result. One could argue that it wouldn't hurt to always return the last "@" occurrance (and not the first), though.

The most up-do-date release I found for libnfsidmap2 is 0.27[6], and that seems to be a self-appointed version done by Redhat. Citi, which is the site mentioned in d/control, still has only 0.25[7]

0.27 does NOT contain the second fix, just the first.

I emailed Steve D. (from that fedorapeople site 0.27 release) asking who is upstream now, he said he doesn't think there is one.

I would need a lot more investigation to understand when the second patch is needed. So given:
- 0.27 does not contain the second fix
- I found no other reference to the second fix "in the wild", just in these bug reports, and no follow-up confirmation about what is fixed (I specifically asked in https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=581199#25)
- the first patch fixes the reported problem scenario
- beta freeze is looming

I'm inclined to upload just with the first patch hunk, and later update it if needed.

1. http://bugs.debian.org/744768, http://bugs.debian.org/581199, http://bugs.debian.org/924425
2. https://bugs.launchpad.net/bugs/1819197, https://bugs.launchpad.net/bugs/1728310, https://bugs.launchpad.net/bugs/584942
3. https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=581199;filename=libnfsidmap_0.20-1_fix_at_sign_user_with_domain.diff;msg=5
4. https://launchpad.net/~ahasenack/+archive/ubuntu/libnfsidmap-krb5-1819197
5. https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=581199;filename=libnfsidmap_0.25-5ubuntu1.debdiff;msg=15
6. https://fedorapeople.org/~steved/libnfsidmap/
7. http://www.citi.umich.edu/projects/nfsv4/linux/

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

I did a codesearch in debian (https://codesearch.debian.net/search?q=nfs4_gss_princ_to_ids) for nfs4_gss_princ_to_ids, and the only non-libnfsidmap package that calls it is nfs-utils. Doesn't tell me yet if that function always uses a well formatted principal name.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote : Posted in a previous version of this proposal

Thank you for the explanation why you picked hunk#1 and not hunk#2 or hunk#1+#2 - that really helped especially given the multitude of bugs involved.
I agree to changing what we know the purpose for any only if needed amend later with the second change.

Also the patch and its integration look good to me.

In the name of the users, thanks for sorting out this old pain point!
+1

The only thing for a coming upload.
This is the bionic branch - the change as proposed (as you know) will need to go to Disco first. Due to Bionic and Cosmic being the same version more expect:
libnfsidmap (0.25-5.1ubuntu0.18.04.1) bionic; urgency=medium
libnfsidmap (0.25-5.1ubuntu0.18.10.1) cosmic; urgency=medium
libnfsidmap (0.25-5.1ubuntu1) disco; urgency=medium

The currently proposed 0.25-5.1ubuntu0.1 will not work that well since you'll have a conflict in Cosmic.

review: Approve
Revision history for this message
Andreas Hasenack (ahasenack) wrote : Posted in a previous version of this proposal

Oh, good catch on the version number, I forgot I was building for bionic to test at home.

fe581f8... by Andreas Hasenack

changelog: fix version number for SRU

Unmerged commits

fe581f8... by Andreas Hasenack

changelog: fix version number for SRU

b5e53e7... by Andreas Hasenack

changelog

a8be1c2... by Andreas Hasenack

  * d/p/03-uid-map-krb5.patch: fix uid mapping when sec=krb5 is used
    (LP: #1819197)

3acb545... by Chris Hofstaedtler

Import patches-unapplied version 0.25-5.1 to debian/sid

Imported using git-ubuntu import.

Changelog parent: 18e7544e77273a60e3ecfc48d1ae355b432d8a88

New changelog entries:
  * Non-maintainer upload.
  * Stop setting compressor explicitly. (Closes: #833241)

18e7544... by Anibal Monsalve Salazar

Import patches-unapplied version 0.25-5 to debian/sid

Imported using git-ubuntu import.

Changelog parent: 5c2ae31acceebb1b02dc5dae7b1fe0be285fe3b7

New changelog entries:
  * Standards Version is 3.9.4
  * Fix various warnings in idmapd.conf.5
    Add 02-idmapd.conf.5.patch
  [ Colin Watson ]
  * Replace incorrect Conflicts on nfs-common with Breaks/Replaces.
    Closes: #654714

5c2ae31... by Anibal Monsalve Salazar

Import patches-unapplied version 0.25-4 to debian/sid

Imported using git-ubuntu import.

Changelog parent: 2b37bc61f36455171bb9c2695b3cc39a2f6bdd79

New changelog entries:
  * Set pluginpath to /lib/$(DEB_HOST_MULTIARCH)/libnfsidmap
    Closes: #676719

2b37bc6... by Anibal Monsalve Salazar

Import patches-unapplied version 0.25-3 to debian/sid

Imported using git-ubuntu import.

Changelog parent: 9946832a6a149a1026605b21966c4bc6fc5bb18f

New changelog entries:
  * Add multiarch support
  * Use hardening options
  * DH compatibility level is 9
  * Fix wrong-section-according-to-package-name

9946832... by Anibal Monsalve Salazar

Import patches-unapplied version 0.25-2 to debian/sid

Imported using git-ubuntu import.

Changelog parent: 511ef49e1f5ea33384f9f68b416a4cefb43a1c5c

New changelog entries:
  * Conflicts with nfs-common (= 1:1.1.2-6lenny2)
    Closes: 654714
  * Build-depends on autoconf (>= 2.68)
    Closes: 660914
  * Fix "umich_ldap.so is an empty module"
    Add 01-661215-wrong-double-ldap-check.patch
    Patch by Steve Langasek
    Closes: 661215
    LP: #939232

511ef49... by Anibal Monsalve Salazar

Import patches-unapplied version 0.25-1 to debian/sid

Imported using git-ubuntu import.

Changelog parent: 3fd64e2ddfe05e2bb44438cd56d9fe4b2b0520e8

New changelog entries:
  * New upstream version 0.25
    Changes since libnfsidmap-0.24
      nss_getpwnam: ignore case when comparing domain names
      rpc.idmapd fails to load plugings
      Cleaned up some warnings
      Ignoring more git files
      Improve debugging during lib initialization
      Added new --with-pluginpath configuration flag
    Delete
      02-add--with-pluginpath-configuration-flag.patch
      03-improve-debugging-during-lib-initialization.patch
      04-clean-up-some-warnings.patch
      05-dont-fail-to-load-plugings.patch
      06-ignore-case-when-comparing-domain-names.patch

3fd64e2... by Anibal Monsalve Salazar

Import patches-unapplied version 0.24-4 to debian/sid

Imported using git-ubuntu import.

Changelog parent: fa9ddf61e820a89f40f328be2d368741bfd707f6

New changelog entries:
  * Add --with-pluginpath configuration flag
    Add 02-add--with-pluginpath-configuration-flag.patch
  * Improve debugging during lib initialization
    Add 03-improve-debugging-during-lib-initialization.patch
  * Clean up some warnings
    Add 04-clean-up-some-warnings.patch
  * Don't fail to load plugings
    Add 05-dont-fail-to-load-plugings.patch
  * Ignore case when comparing domain names
    Add 06-ignore-case-when-comparing-domain-names.patch
  * Set configure flag --with-pluginpath to /lib/libnfsidmap
    Delete 01-649491-move-library.patch

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 07f91f1..dabee0d 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,10 @@
6+libnfsidmap (0.25-5.1ubuntu0.18.04.1) bionic; urgency=medium
7+
8+ * d/p/03-uid-map-krb5.patch: fix uid mapping when sec=krb5 is used
9+ (LP: #1819197)
10+
11+ -- Andreas Hasenack <andreas@canonical.com> Fri, 15 Mar 2019 11:16:24 -0300
12+
13 libnfsidmap (0.25-5.1) unstable; urgency=medium
14
15 * Non-maintainer upload.
16diff --git a/debian/patches/03-uid-map-krb5.patch b/debian/patches/03-uid-map-krb5.patch
17new file mode 100644
18index 0000000..2b07685
19--- /dev/null
20+++ b/debian/patches/03-uid-map-krb5.patch
21@@ -0,0 +1,30 @@
22+From 309a89975a50bf53c408233a1bb5b10fd579ca30 Mon Sep 17 00:00:00 2001
23+From: "Signed-off-by: Shijoe Panjikkaran" <spanjikk@redhat.com>
24+Date: Wed, 30 Apr 2014 13:19:34 -0400
25+Subject: [PATCH] nss: use strrchr() instead of strchr() to get the last
26+ occurrence of "@"
27+
28+Signed-off-by: Shijoe Panjikkaran <spanjikk@redhat.com>
29+Signed-off-by: Steve Dickson <steved@redhat.com>
30+
31+Origin: https://github.com/Distrotech/libnfsidmap/commit/309a89975
32+Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=924425
33+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/libnfsidmap/+bug/1819197
34+Last-Update: 2019-03-15
35+---
36+ nss.c | 2 +-
37+ 1 file changed, 1 insertion(+), 1 deletion(-)
38+
39+diff --git a/nss.c b/nss.c
40+index b2b1227..f8129fe 100644
41+--- a/nss.c
42++++ b/nss.c
43+@@ -135,7 +135,7 @@ static char *strip_domain(const char *name, const char *domain)
44+ char *l = NULL;
45+ int len;
46+
47+- c = strchr(name, '@');
48++ c = strrchr(name, '@');
49+ if (c == NULL && domain != NULL)
50+ goto out;
51+ if (c == NULL && domain == NULL) {
52diff --git a/debian/patches/series b/debian/patches/series
53index 452e741..6a5cb27 100644
54--- a/debian/patches/series
55+++ b/debian/patches/series
56@@ -1,2 +1,3 @@
57 01-661215-wrong-double-ldap-check.patch
58 02-idmapd.conf.5.patch
59+03-uid-map-krb5.patch

Subscribers

People subscribed via source and target branches