On Sat, 2004-02-28 at 02:53, Adam Heath wrote:
> Why did you nmu dpkg? I see no mail from you on the mailing list about
> preparing an nmu.
>
Because the package had Release-Critical bugs open against it which, for
whatever reason, had not been acted upon by the maintainers. All of
these were over a month old.
For such an important package to be in this state right before a release
is not a good situation to be in.
You may have perfectly valid reasons for not being able to work on dpkg
at the moment, I'm not contesting that. However these reasons should
not stop *anyone else* working on dpkg.
You appear to have taken the stance that dpkg is somehow 'sacred' and
not to be touched by the hands of mere mortals not yet blessed; when
instead you should be welcoming the fact there's people out there who
have suddenly got it into their minds to triage, test and fix the
(current) 663 open bugs against it.
And, to put it bluntly, there is a long and honourable history of NMUing
dpkg.
> I stopped receiving @doogie.org mail yesterday afternoon(forgot about my
> domain). If you sent mail after that, then you didn't wait long enough. And,
> besides, I'm not the maintainer of dpkg; this list is.
>
> I see you sent me a *private* mail about. That is not how you discuss dpkg.
> I will not participate in any discussion about dpkg that isn't public. If you
> want to discuss dpkg, then resend your mail publically. I will have *none*
> of these backdoor meetings.
>
It was hardly a backdoor meeting, it has been publically announced and
discussed. Your co-maintainer certainly knew about it, but
unfortunately couldn't attend at the last minute and instead provided
those asked to attend (including myself) with points to start useful
discussion.
And the single, greatest point to come out of this meeting was that dpkg
is, quite frankly, in a terrible state and that it obviously needed
people to work on it.
Of course, you should know all of this already because I mailed a full
and detailed summary of everything we discussed to you and Wichert (the
contents of the dpkg Uploaders: field). It made specific mention of our
intent to upload a version of dpkg to fix the release-critical bugs.
I mailed it to you personally to ensure you read it, as you haven't been
active on debian-dpkg for at least a month and Wichert hasn't in the
past 6 months! For the audience out there, this includes any bug
activity for dpkg.
That mail isn't private, if anyone out there is interested in what we
talked about I'll be happy to open it to a wider audience, I just
thought you might like to talk about it first. Hell, if those people
agree with us, and want to help out our effort to fix dpkg bugs, that'd
be great.
I'd actually like to bring up a point of confusion here as well. You
say that the upload was "unapproved" (in bold capitals, no less);
unapproved by whom? If your statement above that the debian-dpkg list
are the current maintainers of dpkg, then anyone on that list could've
approved it, including myself?
Alternatively, you and Wichert are the dpkg maintainers, in which case I
did send that mail to the right place, and the problem is that you
didn't read it; for which I cannot be blamed.
> When preparing an nmu, one should do the changes, create a diff, file a bug
> with the diff, wait, then upload. These steps *were not done*.
>
I'm so going to hate myself for this, but I'm going to quote policy at
you:
Also, after doing an NMU, you have to open a new bug and include a
patch showing all the changes you have made. Alternatively you can
send that information to the existing bugs that are fixed by your NMU.
-- 5.11.4.3. Source NMUs and the Bug Tracking System
The bugs fixed by this NMU were *ALREADY* marked with the [patch] tag,
except one which was a trivial one line fix that was already fixed on
CVS HEAD.
I believe, in of itself, that this would've sufficed for policy. I went
further than this, and updated each bug with the individual patch
applied, including ChangeLog entry, for easier analysis.
I leave it to the audience whether an NMU to fix Release Critical bugs
that had been ignored by the maintainers is correct, or not. Especially
in the pre-release state we should all be in by now.
> Developers: do not install dpkg 1.10.18.1, until such time as the code has
> been auditted.
>
Put your toys back in your pram, Adam; now you're just being childish.
Do you really believe so poorly of your fellow developers that any
attempt to fix bugs is an attempt to compromise security?
Those people who helped produce this NMU *also* maintain (amongst other
packages): build-essential, base-passwd, openssh, apache, apache2,
mailman, man-db, binfmt-support, libtool & groff.
To be honest, I'm quite offended and indeed, worried, that you would
suggest such a thing.
For those who haven't fled in terror, and are curious, I've attached the
complete diff (before autoconfery) for auditing.
I await to be carted away in handcuffs. In fact, I demand it.
Handcuffs are hot!
Scott
--
Have you ever, ever felt like this?
Had strange things happen? Are you going round the twist?
Index: debian/control
===================================================================
--- debian/control (revision 1665)
+++ debian/control (revision 1840)
@@ -13,7 +13,7 @@
Architecture: any
Essential: yes
Pre-Depends: dselect, ${shlibs:Pre-Depends}
-Conflicts: sysvinit (<< 2.82-1), dpkg-iasearch (<< 0.11), dpkg-static, dpkg-dev (<< 1.9)
+Conflicts: sysvinit (<< 2.82-1), dpkg-iasearch (<< 0.11), dpkg-static, dpkg-dev (<< 1.10)
Replaces: dpkg-doc-ja, dpkg-static, manpages-de (<= 0.4-3)
Description: Package maintenance system for Debian
This package contains the programs which handle the installation and
@@ -31,7 +31,7 @@
Priority: optional
Pre-Depends: dselect, ${shlibs:Pre-Depends}
Replaces: dpkg-doc-ja, dpkg, manpages-de (<= 0.4-3)
-Conflicts: sysvinit (<< 2.82-1), dpkg-iasearch (<< 0.11), dpkg, dpkg-dev (<< 1.9)
+Conflicts: sysvinit (<< 2.82-1), dpkg-iasearch (<< 0.11), dpkg, dpkg-dev (<< 1.10)
Provides: dpkg
Description: Package maintenance system for Debian (static compile)
This package contains the programs which handle the installation and
Index: debian/changelog
===================================================================
--- debian/changelog (revision 1665)
+++ debian/changelog (revision 1840)
@@ -1,3 +1,15 @@
+dpkg (1.10.18.1) unstable; urgency=medium
+
+ * Non-maintainer upload to fix release-critical bugs.
+ * Terminate string buffer in main/remove.c. Closes: #228379.
+ * Prevent stashing of hardlinked devices and setuid or setgid binaries
+ by removing permissions on upgrade as well as on remove.
+ Closes: #225692.
+ * Update dpkg conflicts to << 1.10, instead of 1.9.
+ Closes: #190611, #221989, #222760.
+
+ -- Scott James Remnant <scott@netsplit.com> Thu, 26 Feb 2004 01:17:27 +0000
+
dpkg (1.10.18) unstable; urgency=medium
* Rebuild, tagging and releasing correctly from cvs this time.
Index: autogen.sh
===================================================================
--- autogen.sh (revision 1665)
+++ autogen.sh (revision 1840)
@@ -4,6 +4,6 @@
# Start by setting up everything for main tree
aclocal -I ./automake
-gettextize $copy -f
+gettextize $copy -f -c
autoheader
autoconf
Index: dpkg-deb/main.c
===================================================================
--- dpkg-deb/main.c (revision 1665)
+++ dpkg-deb/main.c (revision 1840)
@@ -54,42 +54,42 @@
}
static void usage(void) {
- if (fputs(_("\
-Command:\n\
- -b|--build <directory> [<deb>] build an archive.\n\
- -c|--contents <deb> list contents.\n\
- -I|--info <deb> [<cfile>...] show info to stdout.\n\
- -W|--show <deb> show information on package(s)\n\
- -f|--field <deb> [<cfield>...] show field(s) to stdout.\n\
- -e|--control <deb> [<directory>] extract control info.\n\
- -x|--extract <deb> <directory> extract files.\n\
- -X|--vextract <deb> <directory> extract & list files.\n\
- --fsys-tarfile <deb> output filesystem tarfile.\n\
- -h|--help display this message.\n\
- --version | --licence show version/licence.\n\
-\n\
-<deb> is the filename of a Debian format archive.\n\
-<cfile> is the name of an administrative file component.\n\
-<cfield> is the name of a field in the main `control' file.\n\
-\n\
-Options:\n\
- --showformat=<format> use alternative format for --show\n\
- -D enable debugging output\n\
- --old, --new select archive format\n\
- --nocheck suppress control file check (build bad package).\n\
- -z# to set the compression when building\n\
-\n\
-Format syntax:\n\
- A format is a string that will be output for each package. The format\n\
- can include the standard escape sequences \\n (newline), \\r (carriage\n\
- return) or \\\\ (plain backslash). Package information can be included\n\
- by inserting variable references to package fields using the ${var[;width]}\n\
- syntax. Fields will be right-aligned unless the width is negative in which\n\
- case left alignment will be used. \n\
-\n\
-Use `dpkg' to install and remove packages from your system, or\n\
-`dselect' for user-friendly package management. Packages unpacked\n\
-using `dpkg-deb --extract' will be incorrectly installed !\n"),
+ if (fputs(_(
+"Command:\n"
+" -b|--build <directory> [<deb>] build an archive.\n"
+" -c|--contents <deb> list contents.\n"
+" -I|--info <deb> [<cfile>...] show info to stdout.\n"
+" -W|--show <deb> show information on package(s)\n"
+" -f|--field <deb> [<cfield>...] show field(s) to stdout.\n"
+" -e|--control <deb> [<directory>] extract control info.\n"
+" -x|--extract <deb> <directory> extract files.\n"
+" -X|--vextract <deb> <directory> extract & list files.\n"
+" --fsys-tarfile <deb> output filesystem tarfile.\n"
+" -h|--help display this message.\n"
+" --version | --licence show version/licence.\n"
+"\n"
+"<deb> is the filename of a Debian format archive.\n"
+"<cfile> is the name of an administrative file component.\n"
+"<cfield> is the name of a field in the main `control' file.\n"
+"\n"
+"Options:\n"
+" --showformat=<format> use alternative format for --show\n"
+" -D enable debugging output\n"
+" --old, --new select archive format\n"
+" --nocheck suppress control file check (build bad package).\n"
+" -z# to set the compression when building\n"
+"\n"
+"Format syntax:\n"
+" A format is a string that will be output for each package. The format\n"
+" can include the standard escape sequences \\n (newline), \\r (carriage\n"
+" return) or \\\\ (plain backslash). Package information can be included\n"
+" by inserting variable references to package fields using the ${var[;width]}\n"
+" syntax. Fields will be right-aligned unless the width is negative in which\n"
+" case left alignment will be used. \n"
+"\n"
+"Use `dpkg' to install and remove packages from your system, or\n"
+"`dselect' for user-friendly package management. Packages unpacked\n"
+"using `dpkg-deb --extract' will be incorrectly installed !\n"),
stdout) < 0) werr("stdout");
}
Index: main/processarc.c
===================================================================
--- main/processarc.c (revision 1665)
+++ main/processarc.c (revision 1840)
@@ -639,6 +639,20 @@
} else
debug(dbg_eachfile, "process_archive: could not stat %s, skipping", fnamevb.buf);
if (donotrm) continue;
+ {
+ /*
+ * If file to remove is a device or s[gu]id, change its mode
+ * so that a malicious user cannot use it even if it's linked
+ * to another file.
+ */
+ struct stat stat_buf;
+ if (stat(fnamevb.buf,&stat_buf)==0) {
+ if (S_ISCHR(stat_buf.st_mode) || S_ISBLK(stat_buf.st_mode))
+ chmod(fnamevb.buf, 0);
+ if (stat_buf.st_mode & (S_ISUID|S_ISGID))
+ chmod(fnamevb.buf, stat_buf.st_mode & ~(S_ISUID|S_ISGID));
+ }
+ }
if (!unlink(fnamevb.buf)) continue;
if (errno == ENOTDIR) continue;
}
Index: main/remove.c
===================================================================
--- main/remove.c (revision 1665)
+++ main/remove.c (revision 1840)
@@ -339,6 +339,7 @@
varbufreset(&fnvb);
varbufaddstr(&fnvb,instdir);
varbufaddstr(&fnvb,namenodetouse(namenode,pkg)->name);
+ varbufaddc(&fnvb,0);
if (!stat(fnvb.buf,&stab) && S_ISDIR(stab.st_mode)) {
debug(dbg_eachfiledetail, "removal_bulk is a directory");
Index: ChangeLog
===================================================================
--- ChangeLog (revision 1665)
+++ ChangeLog (revision 1840)
@@ -1,3 +1,36 @@
+Thu Feb 26 01:48:15 GMT 2004 Scott James Remnant <scott@netsplit.com>
+
+ * main/processarc.c (process_archive): Copy code from main/remove.c
+ to ensure that hardlinks to devices, setuid files or setgid files
+ cannot be stashed away in the hope that they become compromisable
+ in the future. This was handled when removing a package, but not
+ when upgrading one.
+
+Thu Feb 26 01:23:13 GMT 2004 Scott James Remnant <scott@netsplit.com>
+
+ * version-nr: Bump to 1.10.18.1
+
+Mon Feb 23 22:46:21 GMT 2004 Scott James Remnant <scott@netsplit.com>
+
+ * dpkg-deb/main.c: Clean up previous badly applied multiline string
+ patch.
+
+Mon Feb 23 01:30:13 CET 2004 Steinar H. Gunderson <sesse@debian.org>
+
+ * main/remove.c: Terminate string buffer correctly.
+
+Fri Feb 20 10:52:09 CET 2004 Scott James Remnant <scott@netsplit.com>
+
+ * autogen.sh: Tell gettextize to copy its files, not symlink them.
+
+Fri Feb 20 10:22:24 CET 2004 Colin Watson <cjwatson@debian.org>
+
+ * dpkg-deb/main.c: Don't use multiline strings (a removed GCC extension).
+
+Tue Jan 27 20:08:12 CST 2003 Adam Heath <doogie@debian.org>
+
+ * debian/control: Update dpkg conflicts to << 1.10, instead of 1.9.
+
Mon Oct 27 13:39:56 CST 2003 Adam Heath <doogie@debian.org>
* version-nr, debian/changelog: Rebuild, tagging and releasing correctly
Index: version-nr
===================================================================
--- version-nr (revision 1665)
+++ version-nr (revision 1840)
@@ -1 +1 @@
-1.10.18
+1.10.18.1
Attachment:
signature.asc
Description: This is a digitally signed message part