Bug#807442: patch

Discussion:

Bug#807442: patch

(too old to reply)

dann frazier

2015-12-08 22:30:02 UTC

Permalink

Attached.

Hendrik Brueckner

2015-12-09 08:10:01 UTC

Permalink

Hi Dann,

Post by dann frazier
Attached.
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)
+
+
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

Thanks for submitting this patch and the lsluns patch as well. I am about to
open another bug to include the device-mapper helper for zipl and chreipl as
well. They are required for booting from device-mapper devices, for example,
LVM, multipath.

Thanks and kind regards,
Hendrik
--
Hendrik Brueckner
***@linux.vnet.ibm.com | IBM Deutschland Research & Development GmbH
Linux on z Systems Development | Schoenaicher Str. 220, 71032 Boeblingen

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Koederitz
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

Philipp Kern

2015-12-13 15:10:02 UTC

Permalink

Post by dann frazier
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)
+
+
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?

Kind regards and thanks
Philipp Kern

Hendrik Brueckner

2015-12-14 08:40:03 UTC

Permalink

Hi Philipp,

Post by Philipp Kern

Thanks for the feedback. I think that sounds good. I put the s390-tools
owners for their feedback on CC.

Post by Philipp Kern
* 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?

Kind regards,
Hendrik

Hendrik Brueckner

2015-12-14 10:40:02 UTC

Permalink

Post by Philipp Kern

Post by dann frazier
--- 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
+

* 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?

I am not sure what you exactly mean with "user" invoked.
Anyhow, /sbin/ makes sense because the intention of the dbginfo.sh
script is to collect system and debugging information. So it is
important to have it available early (even before /usr becomes
mounted).

I would also go further and would suggest to included it in the
s390-tools udeb package to be available in the debian installer
too. But I would have to check if it runs in the debian installer
environment. I could also imagine to integrate it into the
installation-report module.

Thanks and kind regards,
Hendrik

dann frazier

2016-01-29 23:20:02 UTC

Permalink

Post by Hendrik Brueckner

Post by Philipp Kern

* 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?

I am not sure what you exactly mean with "user" invoked.

I believe that note means that this utility appears to be something
that a user would execute in an interactive terminal vs. e.g. a script
in early boot.

Post by Hendrik Brueckner
Anyhow, /sbin/ makes sense because the intention of the dbginfo.sh
script is to collect system and debugging information. So it is
important to have it available early (even before /usr becomes
mounted).

While this maybe a theoretical use case, I don't think the script has
been designed to run in this environment. As one example, it
calls "/usr/bin/id" to check if it is running as root. It also uses the
"find" command in a number of places which, at least on Debian
systems, is installed in /usr/bin.

Post by Hendrik Brueckner
I would also go further and would suggest to included it in the
s390-tools udeb package to be available in the debian installer
too. But I would have to check if it runs in the debian installer
environment. I could also imagine to integrate it into the
installation-report module.

I'd suggest opening separate bugs for those.

dann frazier

2016-01-29 23:20:02 UTC

Permalink

Post by Philipp Kern

* 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.

Hendrik Brueckner

2016-02-04 08:40:02 UTC

Permalink

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.

Post by dann frazier

Post by Philipp Kern

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.
+
+
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
+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
+ # 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.

Post by dann frazier
+
+ ########################################
+ # 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.
+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
+ 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 ;-)

Also keep in mind that the dbginfo.sh is called from within other programs
that are non-interactive.

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.

Post by dann frazier
++
++
+ #######################################
+ # 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
--
Hendrik Brueckner
***@linux.vnet.ibm.com | IBM Deutschland Research & Development GmbH
Linux on z Systems Development | Schoenaicher Str. 220, 71032 Boeblingen

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Koederitz
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

dann frazier

2016-02-04 20:40:02 UTC

Permalink

Post by Hendrik Brueckner
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.

Post by dann frazier

Post by Philipp Kern

OK.

Post by Hendrik Brueckner

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.

Post by Hendrik Brueckner
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?

Post by Hendrik Brueckner
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

Post by Hendrik Brueckner

Thanks and kind regards,
Hendrik