[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#807442: patch



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.

> + 
> + ########################################
> + # 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 ;-)

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.


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


Reply to: