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 <da...@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 <da...@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 <da...@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 <da...@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 >