Merge autopkgtest-cloud:sil2100/private-testing into autopkgtest-cloud:master

[Łukasz Zemczak]

Add support for running private tests and using private PPAs.

Add support for running private tests and using private PPAs.

[Steve Langasek (vorlon)]

Looks sane in general. A few comments inline. I have not reviewed to verify there are no other places in the code that we could have information leaks.

review: Needs Fixing

[Łukasz Zemczak (sil2100)]

Ok, addressed your comments, this should be good for a final re-review.

Regarding the keying of the piece of code on 'fingerprint' - this is necessary as all of the operations that are being done in that conditional only make sense when we don't provide the PPA fingerprint in the request. Since if the fingerprint is present in the PPA definition, we don't need to query the LP API to get it (and we can't for private PPAs anyway).

Could you take a look now? Thank you!

[Łukasz Zemczak (sil2100)]

Actually I almost forgot about one thing and I'm working on it right now: since the test results are private, the britney ADT report will have SWIFT urls to the results which will not works (as the container is private) - but we can have a small SSO protected wrapper to fetch the results for us in webcontrol.

[Iain Lane (laney)]

Thanks, Łukasz, I think this is a great contribution! I've left you a whole bunch of comments inline.

Additonally, could you please write something for the documentation explaining how this side fits together? That's in docs/, and if you don't know then it's worth knowing that VSCode has nice .rst support - live previews. :-)

Have you run this proposal past the people who will be consuming the feature? I'd like to see a signoff from them I think, to make sure what's been designed works, like having to have a public LP team to view the results.

How are the results going to be downloaded by britney (this uses the HTTP API)? Maybe if we do the X-Auth-Token thing britney could be taught to send that in its requests.

review: Needs Fixing

[Łukasz Zemczak (sil2100)]

Hey! Thank you for the review! I will address some of the immediate issues ASAP, some others I'd still like to discuss. Please see the below inline comment follow ups!

Oh, and +1 on documentation. I'll make sure to work on this this week as a priority. I'll once again run it though the security team, but I already gave them an overview of how things are supposed to work.

[Łukasz Zemczak (sil2100)]

Ok, so I pushed most of the changes that have been requested. Please see previous diff comments for context as I had some counter-comments to some review items.

That being said, I will now actually look into using X-Auth-Token. At first I was not very fond of this idea, but during my time off I actually started thinking: why not? Less credentials and possibly should be faster as well. So yeah, will push something in a bit.

[Łukasz Zemczak (sil2100)]

Ok, this should now be back to being reviewable. As said, please check the previous inline comments from the 19th, but I did change to using X-Auth-Token - even makes me think if maybe I should do the same for the britney parts..? Will look into that!

Added some charm parts but uh, since I never worked with this framework before please give me a sign if I forgot something. I edited it once as I forgot that we have SwiftURL in autopkgtest-cloud.conf already so we shouldn't need to include it in the secrets separately (so only token is needed).

[Iain Lane (laney)]

Really sorry, I think this is my fault for not knowing better. I asked for X-Auth-Token but it seems like those are only valid for 1(?) hour as configured by the cloud. To use these we'd need a way to push new values to the web frontend as necessary and that's probably not feasible really.

I just asked IS if this is possible instead:

  - They generate for us a new user per customer
  - We grant this new user read-only access to private artefacts it should be able to see
  - Creds for this user are the ones used by autopkgtest-web and britney etc

That should at least limit the attack surface to leaking some/all private test results, but not place credentials which have read-write access on the web-accessible frontends.

If you like this, I'll help implement this change, since I feel bad that I led you down an invalid path. :(

[Łukasz Zemczak (sil2100)]

Hey! So this should be good to go then. No worries. So in the end I really liked the X-Auth-Token idea myself, which is why I switched to using that so eagerly. I saw that the token generated by `openstack token issue` is valid only for 1 hour but I assumed, just like with the tempurl stuff, that we can simply set our own expiration date while generating the token. I did not check that though, and as you said it seems to be impossible (as it's hard coded by the server config).

So yeah, as per your request I have reverted back to using full swift credentials (and swiftclient). I'll go and to the same modification on the britney branch side in a moment. I added charm bits to what we already had and cleaned up some of the naming of the config so that it's clear those are web credentials.

I think in this case one thing we might think about in the next iteration is getting rid of the 'swiftuser' request parameter in favor of hard coding that. But on the other hand having it still gives us some more flexibility. I mean, for safety, we could then have 3 sets of swift credentials: the main ones used on the workers (with write access, sensitive), then a main web-frontend read user to be used here by private-results (with full read access everywhere) and then one that we'd hold in the britney instance that would only have read access to the results that it submitted (so passed via 'swiftuser'). We can of course make the two last ones the same user if we don't feel it's too risky too.

[Iain Lane (laney)]

Canonical people can see ticket #131365 for creating a new read-only user for the proxy to use.

Once that's completed, assuming it doesn't take too long, I'll build a charm from this branch and deploy it into staging.

Maybe for the next iteration I'd like us to think about

  - Should we have read only creds per-container? Idea is to try to limit the exposure if we mess up somehow, but not sure how much it matters.
  - As Łukasz says, getting rid of the swiftuser parameter; I feel like it should be able to be inferred somehow from the rest of the request (i.e. basically hardcoded, maybe in configuration in some way).

[Łukasz Zemczak (sil2100)]

Hey! Could someone take a look if this is now good with the new history and get it merged? Thanks!

[Iain Lane (laney)]

Yep, sorry for the delay!

The only think I'm missing is documentation (inside docs/). I'll merge but could you please add some in the next few days?

review: Approve

diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/run-autopkgtest b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/run-autopkgtest
index 5c2bd67..febd6ea 100755
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/run-autopkgtest
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/tools/run-autopkgtest
@@ -82,6 +82,21 @@ def parse_args():
         "Otherwise this has the same behaviour as test-git.",
     )
     parser.add_argument(
+        "--swiftuser",
+        help="Swift username for private result handling. "
+        "The new ADT infrastructure now supports running private tests. "
+        "Such tests will put their rest results in private containers "
+        "only readable by the selected swiftuser.",
+    )
+    parser.add_argument(
+        "--readable-by",
+        action="append",
+        default=[],
+        metavar="LP team/username",
+        help="Make private result readable by the selected LP user/team. "
+        "Can be specified multiple times.",
+    )
+    parser.add_argument(
         "--all-proposed",
         action="store_true",
         help="Disable apt pinning and use all of -proposed",
@@ -146,6 +161,10 @@ if __name__ == "__main__":
     if args.test_bzr:
         params["test-bzr"] = args.test_bzr
         context = "upstream-"
+    if args.swiftuser:
+        params["swiftuser"] = args.swiftuser
+    if args.readable_by:
+        params["readable-by"] = args.readable_by
     if args.all_proposed:
         params["all-proposed"] = True
     try:
diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker
index e8f85eb..8866f2c 100755
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker
@@ -306,6 +306,10 @@ def process_output_dir(dir, pkgname, code, triggers):
     subprocess.check_call(['gzip', '-9', os.path.join(dir, 'log')])
     files.discard('log')
 
+    # the readable-by file, if present, needs to stay intact and be uploaded
+    # to the container as is, as it's used for ACL
+    files.discard('readable-by')
+
     if files:
         # tar up all other artifacts
         subprocess.check_call(['tar', '-czf', 'artifacts.tar.gz'] + list(files), cwd=dir)
@@ -360,26 +364,31 @@ def subst(s, big_package, release, architecture, hostarch, pkgname):
     return s
 
 
-def send_status_info(queue, release, architecture, pkgname, params, out_dir, running, duration):
+def send_status_info(queue, release, architecture, pkgname, params, out_dir, running, duration, private=False):
     '''Send status and logtail to status queue'''
 
     if not queue:
         return
 
-    # print('status_info:', release, architecture, pkgname, out_dir, running)
-    try:
-        with open(os.path.join(out_dir, 'log'), 'rb') as f:
-            try:
-                f.seek(-2000, os.SEEK_END)
-                # throw away the first line as we almost surely cut that out in
-                # the middle
-                f.readline()
-            except IOError:
-                # file is smaller than 2000 bytes? okay
-                pass
-            logtail = f.read().decode('UTF-8', errors='replace')
-    except (IOError, OSError) as e:
-        logtail = 'Cannot read log file: %s' % e
+    if private:
+        pkgname = 'private-test'
+        params = {}
+        logtail = 'Running private test'
+    else:
+        # print('status_info:', release, architecture, pkgname, out_dir, running)
+        try:
+            with open(os.path.join(out_dir, 'log'), 'rb') as f:
+                try:
+                    f.seek(-2000, os.SEEK_END)
+                    # throw away the first line as we almost surely cut that out in
+                    # the middle
+                    f.readline()
+                except IOError:
+                    # file is smaller than 2000 bytes? okay
+                    pass
+                logtail = f.read().decode('UTF-8', errors='replace')
+        except (IOError, OSError) as e:
+            logtail = 'Cannot read log file: %s' % e
 
     msg = json.dumps({'release': release,
                       'architecture': architecture,
@@ -390,8 +399,7 @@ def send_status_info(queue, release, architecture, pkgname, params, out_dir, run
                       'logtail': logtail})
     queue.basic_publish(amqp.Message(msg, delivery_mode=2), status_exchange_name, '')
 
-
-def call_autopkgtest(argv, release, architecture, pkgname, params, out_dir, start_time):
+def call_autopkgtest(argv, release, architecture, pkgname, params, out_dir, start_time, private=False):
     '''Call autopkgtest and regularly send status/logtail to status_exchange_name
 
     Return exit code.
@@ -412,11 +420,13 @@ def call_autopkgtest(argv, release, architecture, pkgname, params, out_dir, star
         status_update_counter = (status_update_counter + 1) % 10
         if status_update_counter == 0:
             send_status_info(status_amqp, release, architecture, pkgname,
-                             params, out_dir, True, int(time.time() - start_time))
+                             params, out_dir, True, int(time.time() - start_time),
+                             private)
 
     ret = autopkgtest.wait()
     send_status_info(status_amqp, release, architecture, pkgname, params,
-                     out_dir, False, int(time.time() - start_time))
+                     out_dir, False, int(time.time() - start_time),
+                     private)
 
     return ret
 
@@ -454,6 +464,7 @@ def request(msg):
     read_per_package_configs(cfg)
 
     dont_run = False
+    private = False
 
     # FIXME: make this more elegant
     fields = msg.delivery_info['routing_key'].split('-')
@@ -565,42 +576,60 @@ def request(msg):
     if 'ppas' in params and params['ppas']:
         for ppa in params['ppas']:
             try:
-                (ppauser, ppaname) = ppa.split('/')
+                (ppacreds, _, ppaurl) = ppa.rpartition('@')
+                (ppaurl, _, fingerprint) = ppaurl.partition(':')
+                (ppacreds_user, ppacreds_pass) = ppacreds.split(':') if ppacreds else (None, None)
+                (ppauser, ppaname) = ppaurl.split('/')
             except ValueError:
-                logging.error('Invalid PPA specification, must be lpuser/ppa_name')
+                logging.error('Invalid PPA specification, must be [user:token@]lpuser/ppa_name[:fingerprint]')
                 msg.channel.basic_ack(msg.delivery_tag)
                 return
-            for retry in range(5):
-                try:
+            if fingerprint:
+                logging.debug('Request states that PPA user %s, name %s has GPG fingerprint %s' % (ppauser, ppaname, fingerprint))
+            else:
+                # Private PPAs require the fingerprint passed through the
+                # request as we can't use the LP API to fetch it.
+                if ppacreds_user:
+                    logging.error('Invalid PPA specification, GPG fingerprint required for private PPAs')
+                    msg.channel.basic_ack(msg.delivery_tag)
+                    return
+                for retry in range(5):
                     try:
                         f = urllib.request.urlopen('https://api.launchpad.net/1.0/~%s/+archive/ubuntu/%s' % (ppauser, ppaname))
                         contents = f.read().decode('UTF-8')
                         f.close()
                         fingerprint = json.loads(contents)['signing_key_fingerprint']
                         logging.debug('PPA user %s, name %s has GPG fingerprint %s' % (ppauser, ppaname, fingerprint))
+                    except (IOError, ValueError, KeyError) as e:
+                        logging.error('Cannot get PPA information: "%s". Consuming the request - it will be left dangling; retry once the problem is resolved.' % e)
+                        msg.channel.basic_ack(msg.delivery_tag)
+                        return
                     except HTTPError as e:
-                        # It's quite common to get 503s from LP; retry a few times.
+                        # It's quite common to get 503s from LP; retry a
+                        # few times.
                         if e.code != 503:
                             raise
                         logging.warning('Got error 503 from launchpad API')
                         time.sleep(10)
                     else:
                         break
-                except (IOError, ValueError, KeyError) as e:
-                    logging.error('Cannot get PPA information: "%s". Consuming the request - it will be left dangling; retry once the problem is resolved.' % e)
+                else:
+                    logging.error('Cannot contact Launchpad to get PPA information. Consuming the request - it will be left dangling; retry once the problem is resolved.')
                     msg.channel.basic_ack(msg.delivery_tag)
                     return
+            if ppacreds_user:
+                # Any run with at least one private PPA needs to be private.
+                private = True
+                ppaprefix = 'https://%s:%s@private-' % (ppacreds_user, ppacreds_pass)
             else:
-                logging.error('Cannot contact Launchpad to get PPA information. Consuming the request - it will be left dangling; retry once the problem is resolved.')
-                msg.channel.basic_ack(msg.delivery_tag)
-                return
+                ppaprefix = 'http://'
             # add GPG key
             argv += ['--setup-commands', 'apt-key adv --keyserver keyserver.ubuntu.com --recv-key ' + fingerprint]
             # add apt source
             argv += ['--setup-commands', 'REL=$(sed -rn "/^(deb|deb-src) .*(ubuntu.com|ftpmaster)/ { s/^[^ ]+ +(\[.*\] *)?[^ ]* +([^ -]+) +.*$/\\2/p; q }" /etc/apt/sources.list); '
-                     'echo "deb http://ppa.launchpad.net/%(u)s/%(p)s/ubuntu $REL main" > /etc/apt/sources.list.d/autopkgtest-%(u)s-%(p)s.list; '
-                     'echo "deb-src http://ppa.launchpad.net/%(u)s/%(p)s/ubuntu $REL main" >> /etc/apt/sources.list.d/autopkgtest-%(u)s-%(p)s.list;' %
-                     {'u': ppauser, 'p': ppaname}]
+                     'echo "deb %(prefix)sppa.launchpad.net/%(u)s/%(p)s/ubuntu $REL main" > /etc/apt/sources.list.d/autopkgtest-%(u)s-%(p)s.list; '
+                     'echo "deb-src %(prefix)sppa.launchpad.net/%(u)s/%(p)s/ubuntu $REL main" >> /etc/apt/sources.list.d/autopkgtest-%(u)s-%(p)s.list;' %
+                     {'prefix': ppaprefix, 'u': ppauser, 'p': ppaname}]
 
         # put results into separate container, named by the last PPA
         container += '-%s-%s' % (ppauser, ppaname)
@@ -714,6 +743,19 @@ def request(msg):
                   host_arch(release, architecture),
                   pkgname).split()
 
+    if 'swiftuser' in params:
+        private = True
+    elif private:
+        # Some combination already marked the run as private, but no
+        # swiftuser user has been specified. This is not valid, as otherwise
+        # no one would be realistically able to read back the results.
+        logging.error('Private autopkgtest run detected but no swiftuser identity provided.')
+        msg.channel.basic_ack(msg.delivery_tag)
+        return
+
+    if private:
+        container = 'private-{}'.format(container)
+
     # run autopkgtest; retry up to three times on tmpfail issues
     if not dont_run:
         global running_test
@@ -723,7 +765,8 @@ def request(msg):
         for retry in range(3):
             retry_start_time = time.time()
             logging.info('Running %s', ' '.join(argv))
-            code = call_autopkgtest(argv, release, architecture, pkgname, params, out_dir, start_time)
+            code = call_autopkgtest(argv, release, architecture, pkgname, params, out_dir, start_time,
+                                    private)
 
             is_failure = code in FAIL_CODES
             files = set(os.listdir(out_dir))
@@ -813,6 +856,13 @@ def request(msg):
         with open(os.path.join(out_dir, 'requester'), 'w') as f:
             f.write('%s\n' % params['requester'])
 
+    if 'readable-by' in params:
+        with open(os.path.join(out_dir, 'readable-by'), 'w') as f:
+            if isinstance(params['readable-by'], list):
+                f.write('\n'.join(params['readable-by']))
+            else:
+                f.write('%s\n' % params['readable-by'])
+
     (testpkg_version, duration, requester) = process_output_dir(out_dir,
                                                                 pkgname,
                                                                 code,
@@ -840,8 +890,12 @@ def request(msg):
         swift_con.get_container(container, limit=1)
     except swiftclient.exceptions.ClientException:
         logging.info('container %s does not exist, creating it', container)
-        # make it publicly readable
-        swift_con.put_container(container, headers={'X-Container-Read': '.rlistings,.r:*'})
+        if private:
+            # private result, share only with swiftuser
+            swift_con.put_container(container, headers={'X-Container-Read': '*:%s' % params['swiftuser']})
+        else:
+            # make it publicly readable
+            swift_con.put_container(container, headers={'X-Container-Read': '.rlistings,.r:*'})
         # wait until it exists
         timeout = 50
         while timeout > 0:
diff --git a/charms/focal/autopkgtest-web/config.yaml b/charms/focal/autopkgtest-web/config.yaml
index 9786139..5b82f55 100644
--- a/charms/focal/autopkgtest-web/config.yaml
+++ b/charms/focal/autopkgtest-web/config.yaml
@@ -19,6 +19,10 @@ options:
     type: string
     default:
     description: "project:user:token github credentials for POSTing to statuses_url"
+  swift-web-credentials:
+    type: string
+    default:
+    description: "SWIFT login credentials for the private-result web frontend"
   https-proxy:
     type: string
     default:
diff --git a/charms/focal/autopkgtest-web/layer.yaml b/charms/focal/autopkgtest-web/layer.yaml
index 2c93ec9..9e2819d 100644
--- a/charms/focal/autopkgtest-web/layer.yaml
+++ b/charms/focal/autopkgtest-web/layer.yaml
@@ -18,3 +18,4 @@ options:
       - python3-distro-info
       - python3-flask
       - python3-flask-openid
+      - python3-swiftclient
diff --git a/charms/focal/autopkgtest-web/reactive/autopkgtest_web.py b/charms/focal/autopkgtest-web/reactive/autopkgtest_web.py
index 7fd1e6b..118d4ed 100644
--- a/charms/focal/autopkgtest-web/reactive/autopkgtest_web.py
+++ b/charms/focal/autopkgtest-web/reactive/autopkgtest_web.py
@@ -22,6 +22,9 @@ GITHUB_SECRETS_PATH = os.path.expanduser("~ubuntu/github-secrets.json")
 GITHUB_STATUS_CREDENTIALS_PATH = os.path.expanduser(
     "~ubuntu/github-status-credentials.txt"
 )
+SWIFT_WEB_CREDENTIALS_PATH = os.path.expanduser(
+    "~ubuntu/swift-web-credentials.conf"
+)
 
 
 @when_not("autopkgtest-web.autopkgtest_web_symlinked")
@@ -193,6 +196,7 @@ def set_up_web_config(apache):
                 ScriptAlias /request.cgi {webcontrol_dir}/request.cgi/
                 ScriptAlias /login {webcontrol_dir}/request.cgi/login
                 ScriptAlias /logout {webcontrol_dir}/request.cgi/logout
+                ScriptAlias /private-results {webcontrol_dir}/private-results.cgi/
                 ScriptAlias / {webcontrol_dir}/browse.cgi/
 
                 {server_name}
@@ -236,6 +240,36 @@ def clear_github_secrets():
         pass
 
 
+@when_all("config.changed.swift-web-credentials",
+          "config.set.swift-web-credentials")
+def write_swift_web_credentials():
+    swift_credentials = config().get("swift-web-credentials")
+
+    with open(SWIFT_WEB_CREDENTIALS_PATH, "w") as f:
+        f.write(swift_credentials)
+
+    try:
+        os.symlink(
+            SWIFT_WEB_CREDENTIALS_PATH,
+            os.path.expanduser("~www-data/swift-web-credentials.conf"),
+        )
+    except FileExistsError:
+        pass
+
+
+@when_not("config.set.swift-web-credentials")
+def clear_swift_web_credentials():
+    try:
+        os.unlink(SWIFT_WEB_CREDENTIALS_PATH)
+    except FileNotFoundError:
+        pass
+
+    try:
+        os.unlink(os.path.expanduser("~www-data/swift-web-credentials.conf"))
+    except FileNotFoundError:
+        pass
+
+
 @when_all(
     "config.changed.github-status-credentials",
     "config.set.github-status-credentials",
diff --git a/charms/focal/autopkgtest-web/webcontrol/cache-amqp b/charms/focal/autopkgtest-web/webcontrol/cache-amqp
index bc5df78..0a231e9 100755
--- a/charms/focal/autopkgtest-web/webcontrol/cache-amqp
+++ b/charms/focal/autopkgtest-web/webcontrol/cache-amqp
@@ -110,7 +110,18 @@ class AutopkgtestQueueContents:
         for r in requests:
             if isinstance(r, bytes):
                 r = r.decode("UTF-8")
-            res.append(r)
+            try:
+                req = r.split(None, 1)
+                if len(req) > 1:
+                    params = json.loads(req[1])
+                else:
+                    params = {}
+                if params.get('private', False):
+                    r = 'private job'
+                res.append(r)
+            except (ValueError, IndexError):
+                logging.error('Received invalid request format "%s"', r)
+                return
         return res
 
     def get_queue_contents(self):
diff --git a/charms/focal/autopkgtest-web/webcontrol/download-all-results b/charms/focal/autopkgtest-web/webcontrol/download-all-results
index 9ceb9e5..6696be7 100755
--- a/charms/focal/autopkgtest-web/webcontrol/download-all-results
+++ b/charms/focal/autopkgtest-web/webcontrol/download-all-results
@@ -22,7 +22,7 @@ import urllib.parse
 import time
 
 from distro_info import UbuntuDistroInfo
-from download.utils import get_test_id, init_db
+from helpers.utils import get_test_id, init_db
 from urllib.request import urlopen
 
 LOGGER = logging.getLogger(__name__)
diff --git a/charms/focal/autopkgtest-web/webcontrol/download-results b/charms/focal/autopkgtest-web/webcontrol/download-results
index a62babe..b8d4188 100755
--- a/charms/focal/autopkgtest-web/webcontrol/download-results
+++ b/charms/focal/autopkgtest-web/webcontrol/download-results
@@ -8,7 +8,7 @@ import socket
 import sqlite3
 import urllib.parse
 
-from download.utils import get_test_id, init_db
+from helpers.utils import get_test_id, init_db
 from urllib.request import urlopen
 
 import amqplib.client_0_8 as amqp
diff --git a/charms/focal/autopkgtest-web/webcontrol/download/__pycache__/__init__.cpython-37.pyc b/charms/focal/autopkgtest-web/webcontrol/download/__pycache__/__init__.cpython-37.pyc
deleted file mode 100644
index 3998d24..0000000
Binary files a/charms/focal/autopkgtest-web/webcontrol/download/__pycache__/__init__.cpython-37.pyc and /dev/null differ
diff --git a/charms/focal/autopkgtest-web/webcontrol/download/__pycache__/utils.cpython-37.pyc b/charms/focal/autopkgtest-web/webcontrol/download/__pycache__/utils.cpython-37.pyc
deleted file mode 100644
index 723b94d..0000000
Binary files a/charms/focal/autopkgtest-web/webcontrol/download/__pycache__/utils.cpython-37.pyc and /dev/null differ
diff --git a/charms/focal/autopkgtest-web/webcontrol/download/__init__.py b/charms/focal/autopkgtest-web/webcontrol/helpers/__init__.py
similarity index 100%
rename from charms/focal/autopkgtest-web/webcontrol/download/__init__.py
rename to charms/focal/autopkgtest-web/webcontrol/helpers/__init__.py
diff --git a/charms/focal/autopkgtest-web/webcontrol/download/utils.py b/charms/focal/autopkgtest-web/webcontrol/helpers/utils.py
similarity index 90%
rename from charms/focal/autopkgtest-web/webcontrol/download/utils.py
rename to charms/focal/autopkgtest-web/webcontrol/helpers/utils.py
index 2069516..36c6c4d 100644
--- a/charms/focal/autopkgtest-web/webcontrol/download/utils.py
+++ b/charms/focal/autopkgtest-web/webcontrol/helpers/utils.py
@@ -1,6 +1,20 @@
 import logging
+import os
 import sqlite3
 
+
+def setup_key(app, path):
+    '''Create or load app.secret_key for cookie encryption.'''
+    try:
+        with open(path, 'rb') as f:
+            app.secret_key = f.read()
+    except FileNotFoundError:
+        key = os.urandom(24)
+        with open(path, 'wb') as f:
+            os.fchmod(f.fileno(), 0o600)
+            f.write(key)
+        app.secret_key = key
+
 def init_db(path):
     '''Create DB if it does not exist, and connect to it'''
 
diff --git a/charms/focal/autopkgtest-web/webcontrol/private-results.cgi b/charms/focal/autopkgtest-web/webcontrol/private-results.cgi
new file mode 100755
index 0000000..9aa5a12
--- /dev/null
+++ b/charms/focal/autopkgtest-web/webcontrol/private-results.cgi
@@ -0,0 +1,11 @@
+#!/usr/bin/env python3
+
+"""Run results app as CGI script """
+
+from wsgiref.handlers import CGIHandler
+
+from private_results.app import app
+
+if __name__ == "__main__":
+    app.config["DEBUG"] = True
+    CGIHandler().run(app)
diff --git a/charms/focal/autopkgtest-web/webcontrol/private_results/app.py b/charms/focal/autopkgtest-web/webcontrol/private_results/app.py
new file mode 100644
index 0000000..368d013
--- /dev/null
+++ b/charms/focal/autopkgtest-web/webcontrol/private_results/app.py
@@ -0,0 +1,190 @@
+"""Test Result Fetcher Flask App"""
+import os
+import sys
+import logging
+import swiftclient
+import configparser
+
+from html import escape
+from flask import (Flask, Response, request, session, redirect,
+                   render_template_string)
+from flask_openid import OpenID
+
+sys.path.append('..')
+
+from helpers.utils import setup_key
+from request.submit import Submit
+
+
+HTML = """
+<!doctype html>
+<html>
+<head>
+<meta charset="utf-8">
+<title>Autopkgtest Private Test Result Fetcher</title>
+</head>
+<body>
+{{ content }}
+</body>
+</html>
+"""
+
+LOGIN = """
+<form action="/private-results/login" method="post">
+<input type="submit" value="Log in with Ubuntu SSO">
+<input type="hidden" name="next" value="{{ next }}">
+</form>
+"""
+
+DENIED = "Unprivileged or unavailable."
+
+
+def swift_get_object(connection, container, path):
+    """Fetch an object from swift."""
+    try:
+        _, contents = connection.get_object(container, path)
+    except swiftclient.exceptions.ClientException as e:
+        logging.error('Failed to fetch %s from container (%s)' %
+                      (path, str(e)))
+        return None
+    return contents
+
+
+def validate_user_path(connection, container, nick, path):
+    """Return true if user is allowed to view files under the given path."""
+    # First we need to check if this result is actually sharable
+    allowed_file = swift_get_object(connection, container, path)
+    if not allowed_file:
+        return False
+    allowed = allowed_file.decode('utf-8').splitlines()
+    # Check if user is allowed
+    # (separate step not to do unnecessary LP API calls)
+    if nick in allowed:
+        return True
+    # Check if user is allowed via team membership
+    for entity in allowed:
+        (code, response) = Submit.lp_request('~%s/participants' % entity, {})
+        if code != 200:
+            logging.error('Unable to validate user %s (%s)' % (nick, code))
+            return False
+        for e in response.get('entries', []):
+            if e.get('name') == nick:
+                return True
+    return False
+
+
+# Initialize app
+PATH = os.path.join(os.getenv('TMPDIR', '/tmp'), 'autopkgtest_webcontrol')
+print(PATH)
+os.makedirs(PATH, exist_ok=True)
+app = Flask('private-results')
+# Keep secret persistent between CGI invocations
+secret_path = os.path.join(PATH, 'secret_key')
+setup_key(app, secret_path)
+oid = OpenID(app, os.path.join(PATH, 'openid'), safe_roots=[])
+# Load configuration
+cfg = configparser.ConfigParser()
+cfg.read(os.path.expanduser('~/swift-web-credentials.conf'))
+# Build swift credentials
+auth_url = cfg.get('swift', 'auth_url')
+if '/v2.0' in auth_url:
+    swift_creds = {
+        'authurl': auth_url,
+        'user': cfg.get('swift', 'username'),
+        'key': cfg.get('swift', 'password'),
+        'tenant_name': cfg.get('swift', 'tenant'),
+        'os_options': {
+            'region_name': cfg.get('swift', 'region_name')
+            },
+        'auth_version': '2.0'
+    }
+else:
+    swift_creds = {
+        'authurl': auth_url,
+        'user': cfg.get('swift', 'username'),
+        'key': cfg.get('swift', 'password'),
+        'os_options': {
+            'region_name': cfg.get('swift', 'region_name'),
+            'project_domain_name': cfg.get('swift', 'project_domain_name'),
+            'project_name': cfg.get('swift', 'project_name'),
+            'user_domain_name': cfg.get('swift', 'user_domain_name')
+            },
+        'auth_version': '3'
+    }
+# Connect to swift
+connection = swiftclient.Connection(**swift_creds)
+
+
+#
+# Flask routes
+#
+
[e-mail]('/', methods=['GET'])
+def index_root():
+    """Handle the main index root, just pure informational."""
+    return render_template_string(
+        HTML, content='Please provide the path to the private result.')
+
+
[e-mail]('/<container>/<series>/<arch>/<group>/<src>/<runid>/<file>',
+           methods=['GET'])
+def index_result(container, series, arch, group, src, runid, file):
+    """Handle all GET requests for private tests."""
+    session.permanent = True
+    session['next'] = escape(request.url)
+    if not container.startswith('private-'):
+        return render_template_string(
+            HTML, content='Limited to private results only.')
+    nick = session.get('nickname')
+    if nick:
+        # Authenticated via SSO, so that's a start
+        parent_path = os.path.join(series, arch, group, src, runid)
+        object_path = os.path.join(parent_path, file)
+        acl_path = os.path.join(parent_path, 'readable-by')
+        if not validate_user_path(connection, container, nick, acl_path):
+            return render_template_string(HTML, content=DENIED), 403
+        # We can pull the result now
+        result = swift_get_object(connection, container, object_path)
+        if result is None:
+            return render_template_string(HTML, content=DENIED), 403
+        if file.endswith('.gz'):
+            content_type = 'text/plain; charset=UTF-8'
+            headers = {'Content-Encoding': 'gzip'}
+            return Response(result, content_type=content_type, headers=headers)
+        else:
+            return result
+    else:
+        # XXX: render_template_string urlencodes its context values, so it's
+        #  not really possible to have 'nested HTML' rendered properly.
+        return HTML.replace("{{ content }}",
+                            render_template_string(LOGIN, **session))
+
+
[e-mail]('/login', methods=['GET', 'POST'])
[e-mail]
+def login():
+    """Initiate OpenID login."""
+    if 'nickname' in session:
+        return redirect(oid.get_next_url())
+    if 'next' in request.form:
+        return oid.try_login(
+            'https://login.ubuntu.com/',
+            ask_for=['nickname'])
+    return redirect('/private-results')
+
+
[e-mail]_login
+def identify(resp):
+    """Complete OpenID login."""
+    session.update(
+        identity_url=resp.identity_url,
+        nickname=resp.nickname,
+    )
+    return redirect(oid.get_next_url())
+
+
[e-mail]('/logout')
+def logout():
+    """Clear user session, logging them out."""
+    session.clear()
+    return redirect(oid.get_next_url())
diff --git a/charms/focal/autopkgtest-web/webcontrol/request/app.py b/charms/focal/autopkgtest-web/webcontrol/request/app.py
index b19945f..66e3539 100644
--- a/charms/focal/autopkgtest-web/webcontrol/request/app.py
+++ b/charms/focal/autopkgtest-web/webcontrol/request/app.py
@@ -9,8 +9,10 @@ from html import escape as _escape
 from flask import Flask, request, session, redirect
 from flask_openid import OpenID
 
+from helpers.utils import setup_key
 from request.submit import Submit
 
+
 # map multiple GET vars to AMQP JSON request parameter list
 MULTI_ARGS = {'trigger': 'triggers',
               'ppa': 'ppas',
@@ -55,19 +57,6 @@ SUCCESS = """
 """
 
 
-def setup_key(path):
-    """Create or load app.secret_key for cookie encryption."""
-    try:
-        with open(path, 'rb') as f:
-            app.secret_key = f.read()
-    except FileNotFoundError:
-        key = os.urandom(24)
-        with open(path, 'wb') as f:
-            os.fchmod(f.fileno(), 0o600)
-            f.write(key)
-        app.secret_key = key
-
-
 def check_github_sig(request):
     """Validate github signature of request.
 
@@ -115,7 +104,7 @@ os.makedirs(PATH, exist_ok=True)
 app = Flask('request')
 # keep secret persistent between CGI invocations
 secret_path = os.path.join(PATH, 'secret_key')
-setup_key(secret_path)
+setup_key(app, secret_path)
 oid = OpenID(app, os.path.join(PATH, 'openid'), safe_roots=[])
 
 
diff --git a/charms/focal/autopkgtest-web/webcontrol/request/submit.py b/charms/focal/autopkgtest-web/webcontrol/request/submit.py
index e1326b1..d48a025 100644
--- a/charms/focal/autopkgtest-web/webcontrol/request/submit.py
+++ b/charms/focal/autopkgtest-web/webcontrol/request/submit.py
@@ -84,6 +84,12 @@ class Submit:
             del kwargs['all-proposed']
         except KeyError:
             pass
+        try:
+            if not kwargs['readable-by']:
+                raise ValueError('Invalid readable-by value')
+            del kwargs['readable-by']
+        except KeyError:
+            pass
         # no other kwargs supported
         if kwargs:
             raise ValueError('Invalid argument %s' % list(kwargs)[0])
diff --git a/mojo/service-bundle b/mojo/service-bundle
index 967112c..c0f7dc4 100644
--- a/mojo/service-bundle
+++ b/mojo/service-bundle
@@ -179,6 +179,7 @@ applications:
 {%- if stage_name == "production" or stage_name == "staging" %}
             github-secrets: include-file://{{local_dir}}/github-secrets.json
             github-status-credentials: include-file://{{local_dir}}/github-status-credentials.txt
+            swift-web-credentials: include-file://{{local_dir}}/swift-web-credentials.conf
             https-proxy: {{ https_proxy }}
             no-proxy: {{ no_proxy }}
 {%- endif %}