[libnss-db] Request to review/accept patch: keep DB files open during iteration (Debian #1101371)

--000000000000cd76b40656f2c1ab
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Aurelien, and others,

I'm writing regarding the libnss-db patch proposed in Debian bug #1101371
[1],
which I've also been working on landing in Ubuntu (LP: #2121543 [2]).

I wanted to check whether you'd be willing to review and accept this patch
into the Debian package.

libnss-db currently closes and re-opens the Berkeley DB file for each entry
during iteration (e.g., `getent passwd`). This is controlled by the
`stayopen`
parameter in db-XXX.c, which is always passed as 0 by glibc, effectively
forcing close/reopen on every entry.

For small DB files this is not noticeable, but in environments with large D=
B
files (e.g., 20,000+ entries), this results in approximately 40x worse
performance compared to keeping the file open.

The Proposed patch is simple, in src/db-XXX.c, change:

    keep_db |=3D stayopen;

to:

    keep_db =3D 1;

This ignores the stayopen parameter and forces the DB file to remain open
during iteration, matching the behavior adopted by glibc itself. Proposed
in [1].

This change aligns libnss-db with how glibc's own NSS modules have handled
stayopen since 2016:

  - glibc's nss_files/files-XXX.c ignores stayopen (since CVE-2014-8121 fix=
)
  - glibc's nss_db/db-XXX.c already uses `keep_db |=3D 1` (always stays ope=
n)
  - nss-pam-ldap similarly ignores stayopen

I submitted this upstream to glibc as well [3][4], but after closer
investigation, I concluded that glibc's nss_db implementation has diverged
significantly from libnss-db and does NOT have this problem; glibc's nss_db
already mmap's the file and tracks the open state correctly, only opening i=
f
state.header is NULL. The issue is specific to the libnss-db Debian package=
,
which uses a different Berkeley DB-based implementation.

CVE-2014-8121: As noted by the original bug reporter (Fran=C3=A7ois Lesueur=
),
the stayopen
behavior is historically linked to CVE-2014-8121 [5]. However, this patch
does NOT
reintroduce that vulnerability:

  - CVE-2014-8121 affected glibc's nss_files (flat text-file NSS module),
     not nss_db or libnss-db (Berkeley DB-based).
  - The CVE was caused by file pointer (FILE*) resets during concurrent
     lookup + iteration, leading to infinite loops. Berkeley DB handles
     have completely different semantics from C FILE* pointers.
  - The fix for CVE-2014-8121 in glibc was to IGNORE stayopen and always
     keep the file open, which is exactly what this patch does for
     libnss-db. The patch follows the same mitigation strategy.
  - glibc's own nss_db module has been using `keep_db |=3D 1` (always open)
     for years without issue.

Therefore, I do not see any risk of regression in relation to CVE-2014-8121=
.

I'm aware that libnss-db has broader challenges in Debian right now:
  - The package has been orphaned since 2011 maintained only through QA
uploads.
  - There is a serious/RC bug (#1121575 [6]) because libnss-db depends on
    libdb5.3 (Berkeley DB 5.3), which is itself unmaintained upstream and
    being removed from Debian (#1117120).
  - There is active work to make the package upstreamable and to address
    the db5.3 dependency, but that effort is going to take considerable
    time to complete.
  - Ongoing effort to include libnss-db from glibc [7] [8].

Given that the larger restructuring work is ongoing and will take a while,
I'd like to see this performance patch merged independently and ahead of
that effort. The patch is minimal (a one-line change), low-risk, and
addresses a real-world performance problem that users are hitting today.
It can be landed in the current codebase without conflicting with any
future db5.3 migration or upstream adoption work.


Given that libnss-db is an orphaned package maintained by the Debian QA
team,
and this is a straightforward performance improvement that aligns with
glibc's
own behavior, I'd like to check if you'd be willing to accept this patch
[1],
and would be able to do NW upload for Debian.

Please let me know if this is possible and/or have any questions for me.

Thanks,
Ponnuvel Palaniyappan


[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=3D1101371
[2] https://bugs.launchpad.net/ubuntu/+source/libnss-db/+bug/2121543
[3] https://sourceware.org/bugzilla/show_bug.cgi?id=3D33558
[4] https://marc.info/?l=3Dglibc-alpha&m=3D176102749124614&w=3D3
[5] https://security-tracker.debian.org/tracker/CVE-2014-8121
[6] https://bugs.debian.org/1121575
[7] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=3D1132249
[8] https://salsa.debian.org/glibc-team/glibc/-/merge_requests/38

--000000000000cd76b40656f2c1ab
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><div dir=3D"ltr">Hi Aurelien, and others,=
<br><br>I&#39;m writing regarding the libnss-db patch proposed in Debian bu=
g #1101371 [1],<br>which I&#39;ve also been working on landing in Ubuntu (L=
P: #2121543 [2]).<br><br>I wanted to check whether you&#39;d be willing to =
review and accept this patch<br>into the Debian package.<br><br>libnss-db c=
urrently closes and re-opens the Berkeley DB file for each entry<br>during =
iteration (e.g., `getent passwd`). This is controlled by the `stayopen`<br>=
parameter in db-XXX.c, which is always passed as 0 by glibc, effectively<br=
>forcing close/reopen on every entry.<br><br>For small DB files this is not=
 noticeable, but in environments with large DB<br>files (e.g., 20,000+ entr=
ies), this results in approximately 40x worse<br>performance compared to ke=
eping the file open.<br><br>The Proposed patch is simple, in src/db-XXX.c, =
change:<br><br>=C2=A0 =C2=A0 keep_db |=3D stayopen;<br><br>to:<br><br>=C2=
=A0 =C2=A0 keep_db =3D 1;<br><br>This ignores the stayopen parameter and fo=
rces the DB file to remain open<br>during iteration, matching the behavior =
adopted by glibc itself. Proposed in [1].<br><br>This change aligns libnss-=
db with how glibc&#39;s own NSS modules have handled<br>stayopen since 2016=
:<br><br>=C2=A0 - glibc&#39;s nss_files/files-XXX.c ignores stayopen (since=
 CVE-2014-8121 fix)<br>=C2=A0 - glibc&#39;s nss_db/db-XXX.c already uses `k=
eep_db |=3D 1` (always stays open)<br>=C2=A0 - nss-pam-ldap similarly ignor=
es stayopen<br><br>I submitted this upstream to glibc as well [3][4], but a=
fter closer<br>investigation, I concluded that glibc&#39;s nss_db implement=
ation has diverged<br>significantly from libnss-db and does NOT have this p=
roblem; glibc&#39;s nss_db<br>already mmap&#39;s the file and tracks the op=
en state correctly, only opening if<br>state.header is NULL. The issue is s=
pecific to the libnss-db Debian package,<br>which uses a different Berkeley=
 DB-based implementation.<br><br>CVE-2014-8121: As noted by the original bu=
g reporter (Fran=C3=A7ois Lesueur), the stayopen<br>behavior is historicall=
y linked to CVE-2014-8121 [5]. However, this patch does NOT<br>reintroduce =
that vulnerability:<br><br>=C2=A0 - CVE-2014-8121 affected glibc&#39;s nss_=
files (flat text-file NSS module),<br>=C2=A0 =C2=A0 =C2=A0not nss_db or lib=
nss-db (Berkeley DB-based).<br>=C2=A0 - The CVE was caused by file pointer =
(FILE*) resets during concurrent<br>=C2=A0 =C2=A0 =C2=A0lookup + iteration,=
 leading to infinite loops. Berkeley DB handles<br>=C2=A0 =C2=A0 =C2=A0have=
 completely different semantics from C FILE* pointers.<br>=C2=A0 - The fix =
for CVE-2014-8121 in glibc was to IGNORE stayopen and always<br>=C2=A0 =C2=
=A0 =C2=A0keep the file open, which is exactly what this patch does for<br>=
=C2=A0 =C2=A0 =C2=A0libnss-db. The patch follows the same mitigation strate=
gy.<br>=C2=A0 - glibc&#39;s own nss_db module has been using `keep_db |=3D =
1` (always open)<br>=C2=A0 =C2=A0 =C2=A0for years without issue.<br><br>The=
refore, I do not see any risk of regression in relation to CVE-2014-8121.<b=
r><br>I&#39;m aware that libnss-db has broader challenges in Debian right n=
ow:<br>=C2=A0 - The package has been orphaned since 2011 maintained only th=
rough QA uploads.<br>=C2=A0 - There is a serious/RC bug (#1121575 [6]) beca=
use libnss-db depends on<br>=C2=A0 =C2=A0 libdb5.3 (Berkeley DB 5.3), which=
 is itself unmaintained upstream and<br>=C2=A0 =C2=A0 being removed from De=
bian (#1117120).<br>=C2=A0 - There is active work to make the package upstr=
eamable and to address<br>=C2=A0 =C2=A0 the db5.3 dependency, but that effo=
rt is going to take considerable<br>=C2=A0 =C2=A0 time to complete.<br>=C2=
=A0 - Ongoing effort to include libnss-db from glibc [7] [8].<br><br>Given =
that the larger restructuring work is ongoing and will take a while,<br>I&#=
39;d like to see this performance patch merged independently and ahead of<b=
r>that effort. The patch is minimal (a one-line change), low-risk, and<br>a=
ddresses a real-world performance problem that users are hitting today.<br>=
It can be landed in the current codebase without conflicting with any<br>fu=
ture db5.3 migration or upstream adoption work.<br><br><br>Given that libns=
s-db is an orphaned package maintained by the Debian QA team,<br>and this i=
s a straightforward performance improvement that aligns with glibc&#39;s<br=
>own behavior, I&#39;d like to check if you&#39;d be willing to accept this=
 patch [1],<br>and would be able to do NW upload for Debian.<br><br>Please =
let me know if this is possible and/or have any questions for me.<br><br>Th=
anks,<br>Ponnuvel Palaniyappan<br><br><br>[1] <a href=3D"https://bugs.debia=
n.org/cgi-bin/bugreport.cgi?bug=3D1101371" target=3D"_blank">https://bugs.d=
ebian.org/cgi-bin/bugreport.cgi?bug=3D1101371</a><br>[2] <a href=3D"https:/=
/bugs.launchpad.net/ubuntu/+source/libnss-db/+bug/2121543" target=3D"_blank=
">https://bugs.launchpad.net/ubuntu/+source/libnss-db/+bug/2121543</a><br>[=
3] <a href=3D"https://sourceware.org/bugzilla/show_bug.cgi?id=3D33558" targ=
et=3D"_blank">https://sourceware.org/bugzilla/show_bug.cgi?id=3D33558</a><b=
r>[4] <a href=3D"https://marc.info/?l=3Dglibc-alpha&amp;m=3D176102749124614=
&amp;w=3D3" target=3D"_blank">https://marc.info/?l=3Dglibc-alpha&amp;m=3D17=
6102749124614&amp;w=3D3</a><br>[5] <a href=3D"https://security-tracker.debi=
an.org/tracker/CVE-2014-8121" target=3D"_blank">https://security-tracker.de=
bian.org/tracker/CVE-2014-8121</a><br>[6] <a href=3D"https://bugs.debian.or=
g/1121575" target=3D"_blank">https://bugs.debian.org/1121575</a><br>[7] <a =
href=3D"https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=3D1132249" target=
=3D"_blank">https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=3D1132249</a>=
<br>[8] <a href=3D"https://salsa.debian.org/glibc-team/glibc/-/merge_reques=
ts/38" target=3D"_blank">https://salsa.debian.org/glibc-team/glibc/-/merge_=
requests/38</a><br></div>
</div>
</div>

--000000000000cd76b40656f2c1ab--