Bug 883811 - sosreport includes passwords from KVM und corosync's fence devices
Summary: sosreport includes passwords from KVM und corosync's fence devices
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: sos
Version: 6.3
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: rc
: ---
Assignee: Bryn M. Reeves
QA Contact: David Kutálek
URL:
Whiteboard:
Depends On:
Blocks: 1087515
TreeView+ depends on / blocked
 
Reported: 2012-12-05 11:23 UTC by Benedikt Wildenhain
Modified: 2018-12-03 18:01 UTC (History)
7 users (show)

Fixed In Version: sos-2.2-47.el6
Doc Type: Bug Fix
Doc Text:
Cause: Previous versions of sos did not mask passwords in libvirt's XML configuration files and corosync-obctl command output. Consequence: Passwords may be disclosed to recipients of sosreport data. Fix: These passwords are now elided during collection of sos data. Result: Passwords are not present in libvirt or corosync data collected by sos.
Clone Of:
: 1087515 (view as bug list)
Environment:
Last Closed: 2013-11-21 22:36:47 UTC
Target Upstream Version:


Attachments (Terms of Use)
files in sos_commands with passwords in them (actual passwords removed) (2.32 KB, application/octet-stream)
2012-12-05 12:53 UTC, Benedikt Wildenhain
no flags Details


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2013:1688 normal SHIPPED_LIVE sos bug fix and enhancement update 2013-11-21 00:39:30 UTC

Description Benedikt Wildenhain 2012-12-05 11:23:41 UTC
Description of problem: sosreport includes passwords:
-Libvirt's/KVM's VNC-passwords (/etc/libvirt/qemu/*)
-IPMI fence devices passwords  in output from objctl (sos_commands/corosync/corosync-objctl_-a and sos_commands/cluster/corosync-objctl in created archive)

Version-Release number of selected component (if applicable): 2.2


How reproducible: probably always


Steps to Reproduce:
1. Protect a virtual system by a password
2. Use corosync with a password protected IPMI device
  
Actual results:
Packages files with passwords included

Expected results:
Packages files with passwords blanked out

Comment 2 Bryn M. Reeves 2012-12-05 12:18:44 UTC
Thanks for the report - libvirt problem confirmed for both Spice and VNC display types:

$ grep passwd sosreport-hex.usersys.redhat.com-20121205121027/etc/libvirt/qemu/*.xml
sosreport-hex.usersys.redhat.com-20121205121027/etc/libvirt/qemu/rhel6-vm1.xml:    <graphics type='spice' autoport='yes' passwd='password'/>
sosreport-hex.usersys.redhat.com-20121205121027/etc/libvirt/qemu/rhel6-vm2.xml:    <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='password'>

Would you have an example of the problematic ipmi available? I don't have any ipmi-capable systems readily to hand so I'd have to search one out and get an appropriate configuration set up.

With an example of the text I can probably get a patch out for this today.

Comment 3 Benedikt Wildenhain 2012-12-05 12:53:27 UTC
Created attachment 658168 [details]
files in sos_commands with passwords in them (actual passwords removed)

Actual passwords have been manually replaced by "PASSWORD", names for VMs by "VMNAME", names for cluster nodes by CLUSTERNODENAME1/2.

Comment 4 Bryn M. Reeves 2012-12-05 14:29:47 UTC
Thanks very much! I'll get a patch done for the corosync part shortly.

The libvirt part is now fixed upstream:

commit c6644bf56866e284ec4bbc8dbf64208eed77c6c4
Author: Bryn M. Reeves <bmr@errorists.org>
Date:   Wed Dec 5 14:26:44 2012 +0000

    Add missing import statement to libvirt module

commit bda29f9f42578b374d51ee375136034bb7215baf
Author: Bryn M. Reeves <bmr@errorists.org>
Date:   Wed Dec 5 14:23:03 2012 +0000

    Obscure display passwords in collected libvirt/qemu files

Equivalent patch for 2.2:

commit 2e37c179d75f01e4225246145298385221833d4b
Author: Bryn M. Reeves <bmr@errorists.org>
Date:   Wed Dec 5 14:26:15 2012 +0000

    Obscure display passwords in collected libvirt/qemu files

diff --git a/sos/plugins/libvirt.py b/sos/plugins/libvirt.py
index 001281c..23351c1 100644
--- a/sos/plugins/libvirt.py
+++ b/sos/plugins/libvirt.py
@@ -13,9 +13,17 @@
 ## Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 
 import sos.plugintools
+import glob
+
 class libvirt(sos.plugintools.PluginBase):
     """libvirt-related information
     """
     def setup(self):
         self.addCopySpec("/etc/libvirt/")
         self.addCopySpec("/var/log/libvirt*")
+
+    def postproc(self):
+        for xmlfile in glob.glob("/etc/libvirt/qemu/*.xml"):
+            self.doRegexSub(xmlfile,
+                    r"(\s*passwd=\s*')([^']*)('.*$)",
+                    r"\1******\3")

Comment 5 Bryn M. Reeves 2012-12-05 16:52:23 UTC
I've got something committed on a private branch now that seems to solve the problem for the corosync output. The problem here is that previously we've had no support for applying post-processing substitutions to command output (rather than collected files as handled by doRegexSub()).

I've added a prototype implementation of a method doRegexExtOutputSub() that provides this functionality and hooked it up to a scratch module for testing.

I'll wait to see if there is any feedback upstream on this new interface before committing it in master or pulling it into the rhel-6 branch but it's at least a proof-of-concept:

$ git diff c6644bf56866e284ec4bbc8dbf64208eed77c6c4..HEAD
diff --git a/sos/plugins/__init__.py b/sos/plugins/__init__.py
index bf9f5a0..7375f98 100644
--- a/sos/plugins/__init__.py
+++ b/sos/plugins/__init__.py
@@ -40,6 +40,7 @@ from time import time
 from itertools import *
 import logging
 import urllib2
+import fnmatch
 
 try:
     import json
@@ -164,6 +165,37 @@ class Plugin(object):
         '''Is the package $package_name installed?'''
         return (self.policy().pkgByName(package_name) is not None)
 
+    def doRegexExtOutputSub(self, cmd, regexp, subst):
+        '''Apply a regexp substitution to command output archived by sosreport.
+        cmd is the command name from which output is collected (i.e. excluding
+        parameters). The regexp can be a string or a compiled re object. The
+        substitution string, subst, is a string that replaces each occurrence
+        of regexp in each file collected from cmd. Internally 'cmd' is treated
+        as a glob with a trailing '*' and each matching file from the current
+        module's command list is subjected to the replacement.
+
+        This function returns the number of replacements made.
+        '''
+        globstr = '*' + cmd + '*'
+        cmdpath = os.path.join(self.cInfo['cmddir'], "foo")
+        try:
+            for called in self.executedCommands:
+                if fnmatch.fnmatch(called['exe'], globstr):
+                    path = os.path.join(self.cInfo['cmddir'], called['file'])
+                    readable = self.archive.open_file(path)
+                    result, replacements = re.subn(
+                            regexp, subst, readable.read())
+                    if replacements:
+                        self.archive.add_string(result, path)
+                        return replacements
+                    else:
+                        return 0
+        except Exception, e:
+            return 0
+
+        except Exception, e:
+            print "boo"
+
     def doRegexSub(self, srcpath, regexp, subst):
         '''Apply a regexp substitution to a file archived by sosreport.
         srcpath is the path in the archive where the file can be found.  regexp
diff --git a/sos/utilities.py b/sos/utilities.py
index 9aafacb..2bb8b2e 100644
--- a/sos/utilities.py
+++ b/sos/utilities.py
@@ -25,7 +25,6 @@ from __future__ import with_statement
 import os
 import re
 import string
-import fnmatch
 import inspect
 from stat import *
 #from itertools import *

Comment 6 Bryn M. Reeves 2012-12-05 16:56:23 UTC
This is branch bmr-ext-cmds-postproc btw:

https://github.com/sosreport/sosreport/tree/bmr-ext-cmds-postproc

Comment 7 Bryn M. Reeves 2012-12-05 20:13:48 UTC
Fixed upstream:

commit c3c27264cfecdfdad334a8c08660be5e0590138a
Author: Bryn M. Reeves <bmr@redhat.com>
Date:   Wed Dec 5 19:04:51 2012 +0000

    Obscure password in corosync-objctl output

commit fcee39532f5d6ae49f9f0db429706d2f7085ed2b
Author: Bryn M. Reeves <bmr@redhat.com>
Date:   Wed Dec 5 18:33:35 2012 +0000

    Rename regex substitution functions
    
    Rename doRegexSub and doRegexExtOutputSub to doFileSub and
    doExtOutputSub respectively.

commit 3a982ffb7a63710d8fdbbfb3d885fd6704c8b6fe
Author: Bryn M. Reeves <bmr@errorists.org>
Date:   Wed Dec 5 15:20:34 2012 +0000

    Add implementation of command output post-processing
    
    Add a doRegexExtOutputSub() function to mirror doRegExSub(). This
    allows modukes ti apply an arbitrary regular expression
    substitution to the output collected from external commands.

Comment 8 RHEL Product and Program Management 2012-12-14 08:50:03 UTC
This request was not resolved in time for the current release.
Red Hat invites you to ask your support representative to
propose this request, if still desired, for consideration in
the next release of Red Hat Enterprise Linux.

Comment 12 Benedikt Wildenhain 2013-01-30 12:34:45 UTC
RedHat's Support cannot guarantee that the fix will be included in RHEL 6.5, but considers the inclusion very likely.

Comment 26 errata-xmlrpc 2013-11-21 22:36:47 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2013-1688.html


Note You need to log in before you can comment on or make changes to this bug.