diff mbox

[Branch,~linaro-image-tools/linaro-image-tools/trunk] Rev 514: Hide password for private PPA access: strip passwords from apt sources in the generated hwpack an...

Message ID 20120430161711.1338.14207.launchpad@ackee.canonical.com
State Accepted
Headers show

Commit Message

Fathi Boudra April 30, 2012, 4:17 p.m. UTC
Merge authors:
  Fathi Boudra (fboudra)
Related merge proposals:
  https://code.launchpad.net/~fboudra/linaro-image-tools/bug-987130-lmc-hide-pass/+merge/104013
  proposed by: Fathi Boudra (fboudra)
  review: Approve - James Tunnicliffe (dooferlad)
------------------------------------------------------------
revno: 514 [merge]
committer: Fathi Boudra <fathi.boudra@linaro.org>
branch nick: linaro-image-tools
timestamp: Mon 2012-04-30 19:13:01 +0300
message:
  Hide password for private PPA access: strip passwords from apt sources in the generated hwpack and obfuscate passwords leaked by ConfigParser/FetchFailed exceptions.
modified:
  linaro_image_tools/hwpack/config.py
  linaro_image_tools/hwpack/hardwarepack.py
  linaro_image_tools/hwpack/packages.py
  linaro_image_tools/hwpack/tests/test_hardwarepack.py
  linaro_image_tools/hwpack/tests/test_packages.py


--
lp:linaro-image-tools
https://code.launchpad.net/~linaro-image-tools/linaro-image-tools/trunk

You are subscribed to branch lp:linaro-image-tools.
To unsubscribe from this branch go to https://code.launchpad.net/~linaro-image-tools/linaro-image-tools/trunk/+edit-subscription
diff mbox

Patch

=== modified file 'linaro_image_tools/hwpack/config.py'
--- linaro_image_tools/hwpack/config.py	2011-09-27 07:06:09 +0000
+++ linaro_image_tools/hwpack/config.py	2012-04-29 10:44:55 +0000
@@ -97,8 +97,12 @@ 
 
         :param fp: a file-like object containing the configuration.
         """
-        self.parser = ConfigParser.RawConfigParser()
-        self.parser.readfp(fp)
+        try:
+            self.parser = ConfigParser.RawConfigParser()
+            self.parser.readfp(fp)
+        except ConfigParser.Error, e:
+            obfuscated_e = re.sub(r"([^ ]https://).+?(@)", r"\1***\2", str(e))
+            raise ConfigParser.Error(obfuscated_e)
 
     def validate(self):
         """Check that this configuration follows the schema.

=== modified file 'linaro_image_tools/hwpack/hardwarepack.py'
--- linaro_image_tools/hwpack/hardwarepack.py	2011-09-22 12:23:46 +0000
+++ linaro_image_tools/hwpack/hardwarepack.py	2012-04-29 10:44:55 +0000
@@ -21,6 +21,7 @@ 
 
 import time
 import os
+import urlparse
 
 from linaro_image_tools.hwpack.better_tarfile import writeable_tarfile
 from linaro_image_tools.hwpack.packages import (
@@ -405,9 +406,14 @@ 
                 get_packages_file(
                     [p for p in self.packages if p.content is not None]))
             tf.create_dir(self.SOURCES_LIST_DIRNAME)
+
             for source_name, source_info in self.sources.items():
-                tf.create_file_from_string(
-                    self.SOURCES_LIST_DIRNAME + "/" + source_name + ".list",
-                    "deb " + source_info + "\n")
+                url_parsed = urlparse.urlsplit(source_info)
+
+                # Don't output sources with passwords in them
+                if not url_parsed.password:
+                    tf.create_file_from_string(
+                        self.SOURCES_LIST_DIRNAME + "/" + source_name + ".list",
+                        "deb " + source_info + "\n")
             # TODO: include sources keys etc.
             tf.create_dir(self.SOURCES_LIST_GPG_DIRNAME)

=== modified file 'linaro_image_tools/hwpack/packages.py'
--- linaro_image_tools/hwpack/packages.py	2012-04-20 00:27:08 +0000
+++ linaro_image_tools/hwpack/packages.py	2012-04-29 10:44:55 +0000
@@ -27,8 +27,10 @@ 
 from string import Template
 import subprocess
 import tempfile
+import urlparse
 
 from apt.cache import Cache
+from apt.cache import FetchFailedException
 from apt.package import FetchError
 import apt_pkg
 
@@ -489,9 +491,40 @@ 
         self.set_installed_packages([], reopen=False)
         sources_list = os.path.join(
             self.tempdir, "etc", "apt", "sources.list")
+
         with open(sources_list, 'w') as f:
             for source in self.sources:
+                # To make a file URL look like an HTTP one (for urlparse)
+                # We do this to use urlparse, which is probably more robust
+                # than any regexp we come up with.
+                mangled_source = source
+                if re.search("file:/[^/]", source):
+                    mangled_source = re.sub("file:/", "file://", source)
+
+                url_parsed = urlparse.urlsplit(mangled_source)
+
+                # If the source uses authentication, don't put in sources.list
+                if url_parsed.password:
+                    url_parts_without_user_pass = [url_parsed.scheme,
+                                                   url_parsed.hostname,
+                                                   url_parsed.path,
+                                                   url_parsed.query,
+                                                   url_parsed.fragment]
+
+                    auth_name=os.path.join(
+                        self.tempdir, "etc", "apt", "auth.conf")
+                    with open(auth_name,'w') as auth:
+                        auth.write(
+                            "machine " + url_parsed.hostname + "\n" +
+                            "login " + url_parsed.username + "\n" +
+                            "password " + url_parsed.password + "\n")
+
+                    source = urlparse.urlunsplit(url_parts_without_user_pass)
+
+                    # Get rid of extra / in file URLs
+                    source = re.sub("file://", "file:/", source)
                 f.write("deb %s\n" % source)
+
         if self.architecture is not None:
             apt_conf = os.path.join(self.tempdir, "etc", "apt", "apt.conf")
             with open(apt_conf, 'w') as f:
@@ -510,7 +543,11 @@ 
         apt_pkg.config.set("Dir::bin::dpkg", "/bin/false")
         self.cache = Cache(rootdir=self.tempdir, memonly=True)
         logger.debug("Updating apt cache")
-        self.cache.update()
+        try:
+            self.cache.update()
+        except FetchFailedException, e:
+            obfuscated_e = re.sub(r"([^ ]https://).+?(@)", r"\1***\2", str(e))
+            raise FetchFailedException(obfuscated_e)
         self.cache.open()
         return self
 

=== modified file 'linaro_image_tools/hwpack/tests/test_hardwarepack.py'
--- linaro_image_tools/hwpack/tests/test_hardwarepack.py	2011-08-18 13:46:36 +0000
+++ linaro_image_tools/hwpack/tests/test_hardwarepack.py	2012-04-29 10:44:55 +0000
@@ -24,7 +24,7 @@ 
 import tarfile
 
 from testtools import TestCase
-from testtools.matchers import Equals
+from testtools.matchers import Equals, MismatchError
 
 from linaro_image_tools.hwpack.hardwarepack import HardwarePack, Metadata
 from linaro_image_tools.hwpack.packages import get_packages_file
@@ -560,3 +560,19 @@ 
         self.assertThat(
             tf,
             HardwarePackHasFile("sources.list.d.gpg", type=tarfile.DIRTYPE))
+
+    def test_password_removed_from_urls(self):
+        hwpack = HardwarePack(self.metadata)
+
+        url = "https://username:password@hostname/url precise main"
+        hwpack.add_apt_sources({"protected": url})
+
+        tf = self.get_tarfile(hwpack)
+        try:
+            self.assertThat(
+                tf, HardwarePackHasFile("sources.list.d/protected.list",
+                                        content="deb " + url + "\n"))
+        except MismatchError:
+            pass  # Expect to not find the password protected URL
+        else:
+            self.assertTrue(False, "Found password protected URL in hwpack")

=== modified file 'linaro_image_tools/hwpack/tests/test_packages.py'
--- linaro_image_tools/hwpack/tests/test_packages.py	2011-08-10 22:12:11 +0000
+++ linaro_image_tools/hwpack/tests/test_packages.py	2012-04-29 10:44:55 +0000
@@ -932,6 +932,21 @@ 
                 cache.tempdir, "var", "lib", "dpkg", "status")).read())
 
 
+    def test_package_list_has_no_passwords(self):
+        source1 = self.useFixture(AptSourceFixture([]))
+        path = re.sub("file:/", "file://user:pass@", source1.sources_entry)
+        cache = IsolatedAptCache([path])
+
+        self.addCleanup(cache.cleanup)
+
+        cache.prepare()
+        cache.set_installed_packages([])
+
+        sources_list_location = os.path.join(cache.tempdir, "etc", "apt",
+                                             "sources.list")
+        sources_list = open(sources_list_location).read()
+        self.assertEqual("deb %s\n" % source1.sources_entry, sources_list)
+
 class PackageFetcherTests(TestCaseWithFixtures):
 
     def test_context_manager(self):