Bug#807442: patch
On Thu, Feb 04, 2016 at 09:35:13AM +0100, Hendrik Brueckner wrote:
> Hi Dann,
>
> I have CC'ed Wolfgang who takes care of it from customer service perspective.
> Within our team, we received some feedback for your patches that I want to
> share with you.
>
> On Fri, Jan 29, 2016 at 04:17:32PM -0700, dann frazier wrote:
> > On Sun, Dec 13, 2015 at 03:50:01PM +0100, Philipp Kern wrote:
> > > On Tue, Dec 08, 2015 at 03:17:49PM -0700, dann frazier wrote:
> > > > diff -Nru s390-tools-1.32.0/debian/changelog s390-tools-1.32.0/debian/changelog
> > > > --- s390-tools-1.32.0/debian/changelog 2015-10-25 17:12:02.000000000 +0100
> > > > +++ s390-tools-1.32.0/debian/changelog 2015-12-08 23:14:52.000000000 +0100
> > > > @@ -1,3 +1,9 @@
> > > > +s390-tools (1.32.0-2) UNRELEASED; urgency=medium
> > > > +
> > > > + * Add dbginfo.sh. (Closes: #807442)
> > > > +
> > > > + -- dann frazier <dannf@debian.org> Tue, 08 Dec 2015 22:33:52 +0100
> > > > +
> > > > s390-tools (1.32.0-1) unstable; urgency=medium
> > > >
> > > > * New upstream release
> > > > diff -Nru s390-tools-1.32.0/debian/s390-tools.install s390-tools-1.32.0/debian/s390-tools.install
> > > > --- s390-tools-1.32.0/debian/s390-tools.install 2014-07-26 23:59:18.000000000 +0200
> > > > +++ s390-tools-1.32.0/debian/s390-tools.install 2015-12-08 23:08:30.000000000 +0100
> > > > @@ -10,6 +10,10 @@
> > > > /sbin/dasdview
> > > > /usr/share/man/man8/dasdview.8
> > > >
> > > > +# dbginfo.sh
> > > > +/sbin/dbginfo.sh
> > > > +/usr/share/man/man1/dbginfo.sh.1
> > > > +
> > > > # fdasd
> > > > /sbin/fdasd
> > > > /usr/share/man/man8/fdasd.8
> > >
> > > Three comments:
> > >
> > > * dbginfo.sh should tell the user that the information in the tarball
> > > is sensitive.
> > > * The resulting tarball should be 0600 by default. (The script needs
> > > to run as root anyway, but placing the result world-readable in
> > > /tmp does not seem smart.)
> > > * Unless this is expected to be in /sbin, given that it's user
> > > invoked and not usually scripted, should this be in /usr/sbin
> > > instead?
> >
> > Good feedback, thanks Philipp! I've addressed all 3 issues in the
> > attached updated patch.
>
> > diff -Nru s390-tools-1.32.0/debian/changelog s390-tools-1.32.0/debian/changelog
> > --- s390-tools-1.32.0/debian/changelog 2015-12-13 09:50:48.000000000 -0500
> > +++ s390-tools-1.32.0/debian/changelog 2016-01-29 12:56:29.000000000 -0500
> > @@ -1,3 +1,12 @@
> > +s390-tools (1.32.0-3) UNRELEASED; urgency=medium
> > +
> > + * Add dbginfo.sh. (Closes: #807442, LP: #1539719)
> > + - dbginfo.sh-umask.patch: Avoid leaking content to unprivileged users.
> > + - dbginfo.sh-warn.patch: Warn users about the sensitivity of the data
> > + this tool collects.
> > +
> > + -- dann frazier <dannf@debian.org> Fri, 29 Jan 2016 12:49:16 -0500
> > +
> > s390-tools (1.32.0-2) unstable; urgency=medium
> >
> > [ Hendrik Brueckner ]
> > diff -Nru s390-tools-1.32.0/debian/patches/dbginfo.sh-umask.patch s390-tools-1.32.0/debian/patches/dbginfo.sh-umask.patch
> > --- s390-tools-1.32.0/debian/patches/dbginfo.sh-umask.patch 1969-12-31 19:00:00.000000000 -0500
> > +++ s390-tools-1.32.0/debian/patches/dbginfo.sh-umask.patch 2016-01-29 12:21:06.000000000 -0500
> > @@ -0,0 +1,16 @@
> > +Description: dbginfo.sh: set umask to prevent local leaks of sensitive data
> > +Author: dann frazier <dannf@debian.org>
> > +Last-Update: 2016-01-29
> > +
> > +Index: s390-tools-1.32.0/scripts/dbginfo.sh
> > +===================================================================
> > +--- s390-tools-1.32.0.orig/scripts/dbginfo.sh
> > ++++ s390-tools-1.32.0/scripts/dbginfo.sh
> > +@@ -12,6 +12,7 @@ export LC_ALL
> > + # The general name of this script
> > + readonly SCRIPTNAME="${0##*/}"
> > +
> > ++umask 0077
>
> This is tricky and probaly leads to changed permissions that might be useful
> to detect permissions problem. Wolfgang and team worked on this topic and
> a problem fix will be provided with the next s390-tools version. The idea
> here is to change the permission of the directory which will be created to
> contain all service data.
OK.
> > +
> > + ########################################
> > + # print version info
> > diff -Nru s390-tools-1.32.0/debian/patches/dbginfo.sh-warn.patch s390-tools-1.32.0/debian/patches/dbginfo.sh-warn.patch
> > --- s390-tools-1.32.0/debian/patches/dbginfo.sh-warn.patch 1969-12-31 19:00:00.000000000 -0500
> > +++ s390-tools-1.32.0/debian/patches/dbginfo.sh-warn.patch 2016-01-29 12:32:51.000000000 -0500
> > @@ -0,0 +1,38 @@
> > +Description: dbginfo.sh: Sensitivity training
> > + Warn users that the archive this tool generates contains sensitive data,
> > + and give them an opportunity to exit.
> > +Author: dann frazier <dannf@debian.org>
> > +Last-Update: 2016-01-29
> > +
> > +Index: s390-tools-1.32.0/scripts/dbginfo.sh
> > +===================================================================
> > +--- s390-tools-1.32.0.orig/scripts/dbginfo.sh
> > ++++ s390-tools-1.32.0/scripts/dbginfo.sh
> > +@@ -71,6 +71,27 @@ if test "$(/usr/bin/id -u 2>/dev/null)"
> > + exit 1
> > + fi
> > +
> > ++echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
> > ++echo " Warning: The archive created by this utility will contain sensitive"
> > ++echo " information including, but not limited to:"
> > ++echo " - configuration files"
> > ++echo " - log files"
> > ++echo " - hardware state information"
> > ++echo " - running process state and command line arguments"
> > ++echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
> > ++echo ""
> > ++echo -n " Do you wish to continue? [y/N]> "
> > ++read resp
> > ++case "$resp" in
> > ++ y|Y)
> > ++ :
> > ++ ;;
> > ++ *)
> > ++ echo "OK, exiting."
> > ++ exit 0
> > ++esac
>
> The dbginfo.sh must be started as root and typically whoever acts as root
> should know what it doesn... if not, well, it should be not root ;-)
Hi Henrik,
I don't know that we can expect every user with root privileges to
know what /this/ tool does. Certainly they could inspect the tarball
before distributing it - but it is quite a bit of data for someone who
maybe urgently trying to get a fix for their system. I personally
don't see the harm in a "hey buddy, watch what you do with this"
message - it may make someone rethink e.g. putting it on a public
webserver vs. finding a secure transport mechanism to their support
org.
> Also keep in mind that the dbginfo.sh is called from within other programs
> that are non-interactive.
Yeah - my thought there was that we could add a commandline argument
to tell it to run in non-interactive mode. I omitted that in
this patch because I didn't want to introduce an argument that may
later conflict (or differ) with something upstream.
But, on further thought, I can see why adding such a facility would
be a problem for existing users. If they already have this scripted,
an update shouldn't start making those scripts go interactive.
Perhaps we could just emit a warning at the very end of the output?
> For clarity, what exactly do you understand of "sensitive" data. dbginfo.sh
> does not collect file that contains passwords. If think that dbginfo.sh
> includes password-sensitive data, feel free to report the problem to us.
What a user considers sensitive will vary from user to user. I won't
argue that dbginfo.sh should stop collecting anything that maybe
perceived as sensitive - that would only limit its value as a debug
tool.
But, since you asked, some examples of things that *I* would consider
sensitive (but certainly useful for debug) are:
- firewall configuration/iptables (oh, look at that open port!)
- SW versions (oh, they're running a kernel w/ a known vulnerability!)
- Network config (oh, that's the DNS server to target for MITM!)
- The list of running processes (oh - CorpFoo uses BlahDB!)
-dann
>
> > ++
> > ++
> > + #######################################
> > + # Parsing the command line
> > + #
> > diff -Nru s390-tools-1.32.0/debian/patches/series s390-tools-1.32.0/debian/patches/series
> > --- s390-tools-1.32.0/debian/patches/series 2015-12-13 09:41:14.000000000 -0500
> > +++ s390-tools-1.32.0/debian/patches/series 2016-01-29 12:21:21.000000000 -0500
> > @@ -6,3 +6,5 @@
> > zipl-optional.patch
> > disable.patch
> > sg3-utils.patch
> > +dbginfo.sh-umask.patch
> > +dbginfo.sh-warn.patch
> > diff -Nru s390-tools-1.32.0/debian/s390-tools.install s390-tools-1.32.0/debian/s390-tools.install
> > --- s390-tools-1.32.0/debian/s390-tools.install 2015-12-13 09:47:24.000000000 -0500
> > +++ s390-tools-1.32.0/debian/s390-tools.install 2016-01-29 12:40:00.000000000 -0500
> > @@ -10,6 +10,10 @@
> > /sbin/dasdview
> > /usr/share/man/man8/dasdview.8
> >
> > +# dbginfo.sh
> > +/sbin/dbginfo.sh /usr/sbin
> > +/usr/share/man/man1/dbginfo.sh.1
> > +
> > # fdasd
> > /sbin/fdasd
> > /usr/share/man/man8/fdasd.8
>
> Thanks and kind regards,
> Hendrik
>
Reply to: