From patchwork Mon Apr 30 16:17:11 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Fathi Boudra X-Patchwork-Id: 8313 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id E005823E49 for ; Mon, 30 Apr 2012 16:17:14 +0000 (UTC) Received: from mail-iy0-f180.google.com (mail-iy0-f180.google.com [209.85.210.180]) by fiordland.canonical.com (Postfix) with ESMTP id 81378A180A5 for ; Mon, 30 Apr 2012 16:17:14 +0000 (UTC) Received: by iage36 with SMTP id e36so5988412iag.11 for ; Mon, 30 Apr 2012 09:17:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-forwarded-to:x-forwarded-for:delivered-to:received-spf :content-type:mime-version:x-launchpad-project:x-launchpad-branch :x-launchpad-message-rationale:x-launchpad-branch-revision-number :x-launchpad-notification-type:to:from:subject:message-id:date :reply-to:sender:errors-to:precedence:x-generated-by :x-launchpad-hash:x-gm-message-state; bh=qjOCofHIepBcAIFqO/K4naLQ+D+rrI73JzSMPgksxpg=; b=e2xRtw41+xsln3yMTxEivUFrWivKipI/1cX+afMiJkos1VueeSgEidv6JTGNmVZyHo utUpuTNT/ZApa0mFcUlbQkWKX/4pzPRlTV9DnyZNbOhn3hHZcrjSr+g78cxTsrQkxoM7 NUtW96BdGla+8eTuTF+4C+873awS2pPQAvQvxXT758rKAf71l621V7p11JcYZ1bOXe9J 7a4VGR2MMh2MSrvkKu8+1765MGel6a4QByqtmVD901JXoxhEmWJQyRhIHAZUH65NdMMu +MX8DPtySua5eDPw/QQhjINLbhUWItAcxbr8oOjxtT1mLfj1Qm3nArhYusGpQG0tNv/l n+Aw== Received: by 10.50.57.129 with SMTP id i1mr10959690igq.33.1335802633869; Mon, 30 Apr 2012 09:17:13 -0700 (PDT) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.231.137.198 with SMTP id x6csp123802ibt; Mon, 30 Apr 2012 09:17:12 -0700 (PDT) Received: by 10.180.100.230 with SMTP id fb6mr22206410wib.3.1335802631695; Mon, 30 Apr 2012 09:17:11 -0700 (PDT) Received: from indium.canonical.com (indium.canonical.com. [91.189.90.7]) by mx.google.com with ESMTPS id x8si18429197weq.7.2012.04.30.09.17.11 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 30 Apr 2012 09:17:11 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of bounces@canonical.com designates 91.189.90.7 as permitted sender) client-ip=91.189.90.7; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of bounces@canonical.com designates 91.189.90.7 as permitted sender) smtp.mail=bounces@canonical.com Received: from ackee.canonical.com ([91.189.89.26]) by indium.canonical.com with esmtp (Exim 4.71 #1 (Debian)) id 1SOtHj-0006UA-4e for ; Mon, 30 Apr 2012 16:17:11 +0000 Received: from ackee.canonical.com (localhost [127.0.0.1]) by ackee.canonical.com (Postfix) with ESMTP id 1431DE1AB9 for ; Mon, 30 Apr 2012 16:17:11 +0000 (UTC) MIME-Version: 1.0 X-Launchpad-Project: linaro-image-tools X-Launchpad-Branch: ~linaro-image-tools/linaro-image-tools/trunk X-Launchpad-Message-Rationale: Subscriber X-Launchpad-Branch-Revision-Number: 514 X-Launchpad-Notification-Type: branch-revision To: Linaro Patch Tracker From: noreply@launchpad.net Subject: [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> Date: Mon, 30 Apr 2012 16:17:11 -0000 Reply-To: noreply@launchpad.net Sender: bounces@canonical.com Errors-To: bounces@canonical.com Precedence: bulk X-Generated-By: Launchpad (canonical.com); Revision="15171"; Instance="launchpad-lazr.conf" X-Launchpad-Hash: 06676d1b132e1b8c10ee4db1654a75f4dfe9548f X-Gm-Message-State: ALoCoQnW3v4hpyngPuJ3KgQilQcvrgqJH5hImNjEqC9V1veOlAJin5M7yRfJ4hXyHjGaxFEDCcK1 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 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 === 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):