Issue 6291 - Use client certificate for Windows Store uploads

sitescripts/extensions/bin/createNightlies.py

 # This file is part of the Adblock Plus web scripts,
 # Copyright (C) 2006-present eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 #
 # You should have received a copy of the GNU General Public License
 # along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
 
 """
 
 Nightly builds generation script
 ================================
 
   This script generates nightly builds of extensions, together
   with changelogs and documentation.
 
 """
 
 import argparse
 import ConfigParser
 import binascii
 import base64
-import datetime
 import hashlib
 import hmac
 import json
 import logging
 import os
 import pipes
-import random
 import shutil
 import struct
 import subprocess
 import sys
 import tempfile
 import time
 import uuid
 from urllib import urlencode
 import urllib2
 import urlparse
 import zipfile
 import contextlib
+from xml.dom.minidom import parse as parseXml
 
 from Crypto.PublicKey import RSA
 from Crypto.Signature import PKCS1_v1_5
 import Crypto.Hash.SHA256
 
-from xml.dom.minidom import parse as parseXml
-
 from sitescripts.extensions.utils import (
     compareVersions, Configuration,
-    writeAndroidUpdateManifest
+    writeAndroidUpdateManifest,
 )
 from sitescripts.utils import get_config, get_template
 
 MAX_BUILDS = 50
 
 
 # Google and Microsoft APIs use HTTP error codes with error message in
 # body. So we add the response body to the HTTPError to get more
 # meaningful error messages.
 class HTTPErrorBodyHandler(urllib2.HTTPDefaultErrorHandler):
@@ skipping 28 matching lines @@
 
     def hasChanges(self):
         return self.revision != self.previousRevision
 
     def getCurrentRevision(self):
         """
             retrieves the current revision ID from the repository
         """
         command = [
             'hg', 'id', '-i', '-r', self.config.revision, '--config',
-            'defaults.id=', self.config.repository
+            'defaults.id=', self.config.repository,
         ]
         return subprocess.check_output(command).strip()
 
     def getCurrentBuild(self):
         """
             calculates the (typically numerical) build ID for the current build
         """
         command = ['hg', 'id', '-n', '--config', 'defaults.id=', self.tempdir]
         build = subprocess.check_output(command).strip()
         return build
 
     def getChanges(self):
         """
           retrieve changes between the current and previous ("first") revision
         """
         command = [
             'hg', 'log', '-R', self.tempdir, '-r',
             'reverse(ancestors({}))'.format(self.config.revision), '-l', '50',
             '--encoding', 'utf-8', '--template',
             '{date|isodate}\\0{author|person}\\0{rev}\\0{desc}\\0\\0',
-            '--config', 'defaults.log='
+            '--config', 'defaults.log=',
         ]
         result = subprocess.check_output(command).decode('utf-8')
 
         for change in result.split('\x00\x00'):
             if change:
                 date, author, revision, description = change.split('\x00')
                 yield {'date': date, 'author': author, 'revision': revision, 'description': description}
 
     def copyRepository(self):
         """
@@ skipping 141 matching lines @@
             os.makedirs(baseDir)
 
         # ABP for Android used to have its own update manifest format. We need to
         # generate both that and the new one in the libadblockplus format as long
         # as a significant amount of users is on an old version.
         if self.config.type == 'android':
             newManifestPath = os.path.join(baseDir, 'update.json')
             writeAndroidUpdateManifest(newManifestPath, [{
                 'basename': self.basename,
                 'version': self.version,
-                'updateURL': self.updateURL
+                'updateURL': self.updateURL,
             }])
 
         template = get_template(get_config().get('extensions', templateName),
                                 autoescape=autoescape)
         template.stream({'extensions': [self]}).dump(manifestPath)
 
     def writeIEUpdateManifest(self, versions):
         """
           Writes update.json file for the latest IE build
         """
         if len(versions) == 0:
             return
 
         version = versions[0]
         packageName = self.basename + '-' + version + self.config.packageSuffix
         updateURL = urlparse.urljoin(self.config.nightliesURL, self.basename + '/' + packageName + '?update')
         baseDir = os.path.join(self.config.nightliesDirectory, self.basename)
         manifestPath = os.path.join(baseDir, 'update.json')
 
         from sitescripts.extensions.utils import writeIEUpdateManifest as doWrite
         doWrite(manifestPath, [{
             'basename': self.basename,
             'version': version,
-            'updateURL': updateURL
+            'updateURL': updateURL,
         }])
 
         for suffix in ['-x86.msi', '-x64.msi', '-gpo-x86.msi', '-gpo-x64.msi']:
             linkPath = os.path.join(baseDir, '00latest%s' % suffix)
             outputPath = os.path.join(baseDir, self.basename + '-' + version + suffix)
             self.symlink_or_copy(outputPath, linkPath)
 
     def build(self):
         """
           run the build command in the tempdir
         """
         baseDir = os.path.join(self.config.nightliesDirectory, self.basename)
         if not os.path.exists(baseDir):
             os.makedirs(baseDir)
         outputFile = '%s-%s%s' % (self.basename, self.version, self.config.packageSuffix)
         self.path = os.path.join(baseDir, outputFile)
         self.updateURL = urlparse.urljoin(self.config.nightliesURL, self.basename + '/' + outputFile + '?update')
 
         if self.config.type == 'android':
             apkFile = open(self.path, 'wb')
 
             try:
                 try:
                     port = get_config().get('extensions', 'androidBuildPort')
                 except ConfigParser.NoOptionError:
                     port = '22'
                 command = ['ssh', '-p', port, get_config().get('extensions', 'androidBuildHost')]
                 command.extend(map(pipes.quote, [
                     '/home/android/bin/makedebugbuild.py', '--revision',
-                    self.buildNum, '--version', self.version, '--stdout'
+                    self.buildNum, '--version', self.version, '--stdout',
                 ]))
                 subprocess.check_call(command, stdout=apkFile, close_fds=True)
             except:
                 # clear broken output if any
                 if os.path.exists(self.path):
                     os.remove(self.path)
                 raise
         else:
             env = os.environ
             spiderMonkeyBinary = self.config.spiderMonkeyBinary
             if spiderMonkeyBinary:
                 env = dict(env, SPIDERMONKEY_BINARY=spiderMonkeyBinary)
 
             command = [os.path.join(self.tempdir, 'build.py')]
-            if self.config.type in {'safari', 'edge'}:

[tlucas, 2018/04/13 13:06:04]

The branch 'edge' is built from still uses the old version of buildtools, pre
#6021

[Sebastian Noack, 2018/04/14 02:47:30]

I would rather see a dependency update landing in the "edge" branch. That way we
won't have to bother about changing the code here again, once we merge "master"
back into the "edge" branch.

[tlucas, 2018/04/14 08:55:46]

Yeah, i agree.
@Ollie - what do you think about this? I guess we'll have a separate issue for
that?

[Sebastian Noack, 2018/04/14 09:40:54]

Alternatively, we could add a hack to build.py:

  args = sys.argv[:]
  if '-t' in args:
      index_opt = args.index('-t')
      index_val = index_opt + 1
      if index_val < len(args):
          args.insert(1, args.pop(index_opt))
          args.insert(2, args.pop(index_val))
  buildtools.build.processArgs(BASE_DIR, *args)

This might be less problematic than a dependency update which might require more
changes that will be redundant once we merge "master" back into "edge".

We could even apply the same hack in the "safari" branch and remove this code
path here altogether, and also avoid confusion about inconsistent expected order
of arguments when using build.py during development and release.

[Oleksandr, 2018/04/16 04:37:07]

I would rather merge `master` into `edge` first. That's what my
https://hg.adblockplus.org/buildtools/rev/a3db4a1a49e8 was for. I don't think we
need an issue for that. I have merged as a `Noissue` commit before. There's
currently no conflicts, however the build still fails on Windows because
adblockplusui guys decided they can rely on existence of `bash` everywhere :/
See https://issues.adblockplus.org/ticket/6587

[Sebastian Noack, 2018/04/16 05:45:50]

Wouldn't this rather be a reason to go with my suggested workaround for now?
So that we can move forward here, and just remove it again, once issue 6587 got
sorted, the adblockplusui and buildtools dependencies got updated, and "master"
got merged back into "next".

[Sebastian Noack, 2018/04/16 05:47:19]

Sorry, I meant "edge" (not "next").

[tlucas, 2018/04/16 10:06:07]

All of the above does IMHO encourage a workaround - and i guess
adblockpluschrome @edge is the right place to do it. I'll file a review soon,
thank you!

[tlucas, 2018/04/16 10:25:14]

https://codereview.adblockplus.org/29753557/
https://codereview.adblockplus.org/29753560/

[tlucas, 2018/04/16 14:50:09]

Done.

-                command.extend(['-t', self.config.type, 'build'])
-            else:
-                command.extend(['build', '-t', self.config.type])
-            command.extend(['-b', self.buildNum])
+            command.extend(['build', '-t', self.config.type, '-b',
+                            self.buildNum])
 
             if self.config.type not in {'gecko', 'edge'}:
                 command.extend(['-k', self.config.keyFile])
             command.append(self.path)
             subprocess.check_call(command, env=env)
 
         if not os.path.exists(self.path):
             raise Exception("Build failed, output file hasn't been created")
 
         if self.config.type not in self.downloadable_repos:
@@ skipping 37 matching lines @@
             packageFile = self.basename + '-' + version + self.config.packageSuffix
             changelogFile = self.basename + '-' + version + '.changelog.xhtml'
             if not os.path.exists(os.path.join(baseDir, packageFile)):
                 # Oops
                 continue
 
             link = {
                 'version': version,
                 'download': packageFile,
                 'mtime': os.path.getmtime(os.path.join(baseDir, packageFile)),
-                'size': os.path.getsize(os.path.join(baseDir, packageFile))
+                'size': os.path.getsize(os.path.join(baseDir, packageFile)),
             }
             if os.path.exists(os.path.join(baseDir, changelogFile)):
                 link['changelog'] = changelogFile
             links.append(link)
         template = get_template(get_config().get('extensions', 'nightlyIndexPage'))
         template.stream({'config': self.config, 'links': links}).dump(outputPath)
 
     def read_downloads_lockfile(self):
         path = get_config().get('extensions', 'downloadLockFile')
         try:
@@ skipping 24 matching lines @@
         try:
             for i, entry in enumerate(current[platform]):
                 if entry[filter_key] == filter_value:
                     del current[platform][i]
                 if len(current[platform]) == 0:
                     del current[platform]
         except KeyError:
             pass
         self.write_downloads_lockfile(current)
 
-    def generate_jwt_request(self, issuer, secret, url, method, data=None,
-                             add_headers=[]):
-        header = {
-            'alg': 'HS256',     # HMAC-SHA256
-            'typ': 'JWT',
+    def azure_jwt_signature_fnc(self):
+        return (
+            'RS256',
+            lambda s, m: PKCS1_v1_5.new(s).sign(Crypto.Hash.SHA256.new(m)),
+        )
+
+    def mozilla_jwt_signature_fnc(self):
+        return (
+            'HS256',
+            lambda s, m: hmac.new(s, msg=m, digestmod=hashlib.sha256).digest(),
+        )
+
+    def sign_jwt(self, issuer, secret, url, signature_fnc, jwt_headers={}):
+        alg, fnc = signature_fnc()
+
+        header = {'typ': 'JWT'}
+        header.update(jwt_headers)
+        header.update({'alg': alg})
+
+        issued = int(time.time())
+        expires = issued + 60
+
+        payload = {
+            'aud': url,
+            'iss': issuer,
+            'sub': issuer,
+            'jti': str(uuid.uuid4()),
+            'iat': issued,
+            'nbf': issued,
+            'exp': expires,
         }
 
-        issued = int(time.time())
-        payload = {
-            'iss': issuer,
-            'jti': random.random(),
-            'iat': issued,
-            'exp': issued + 60,
-        }
-
-        hmac_data = '{}.{}'.format(
-            base64.b64encode(json.dumps(header)),
-            base64.b64encode(json.dumps(payload))
-        )
-
-        signature = hmac.new(secret, msg=hmac_data,
-                             digestmod=hashlib.sha256).digest()
-        token = '{}.{}'.format(hmac_data, base64.b64encode(signature))
+        segments = [base64.urlsafe_b64encode(json.dumps(header)),
+                    base64.urlsafe_b64encode(json.dumps(payload))]
+
+        signature = fnc(secret, b'.'.join(segments))
+        segments.append(base64.urlsafe_b64encode(signature))
+        return b'.'.join(segments)
+
+    def generate_mozilla_jwt_request(self, issuer, secret, url, method,
+                                     data=None, add_headers=[]):
+        signed = self.sign_jwt(issuer, secret, url,
+                               self.mozilla_jwt_signature_fnc)
 
         request = urllib2.Request(url, data)
-        request.add_header('Authorization', 'JWT ' + token)
+        request.add_header('Authorization', 'JWT ' + signed)
         for header in add_headers:
             request.add_header(*header)
         request.get_method = lambda: method
 
         return request
 
     def uploadToMozillaAddons(self):
         import urllib3
 
         config = get_config()
 
         upload_url = ('https://addons.mozilla.org/api/v3/addons/{}/'
                       'versions/{}/').format(self.extensionID, self.version)
 
         with open(self.path, 'rb') as file:
             data, content_type = urllib3.filepost.encode_multipart_formdata({
                 'upload': (
                     os.path.basename(self.path),
                     file.read(),
-                    'application/x-xpinstall'
-                )
+                    'application/x-xpinstall',
+                ),
             })
 
-        request = self.generate_jwt_request(
+        request = self.generate_mozilla_jwt_request(
             config.get('extensions', 'amo_key'),
             config.get('extensions', 'amo_secret'),
             upload_url,
             'PUT',
             data,
-            [('Content-Type', content_type)]
+            [('Content-Type', content_type)],
         )
 
         try:
             urllib2.urlopen(request).close()
         except urllib2.HTTPError as e:
             try:
                 logging.error(e.read())
             finally:
                 e.close()
             raise
 
         self.add_to_downloads_lockfile(
             self.config.type,
             {
                 'buildtype': 'devbuild',
                 'app_id': self.extensionID,
                 'version': self.version,
-            }
+            },
         )
         os.remove(self.path)
 
     def download_from_mozilla_addons(self, buildtype, version, app_id):
         config = get_config()
         iss = config.get('extensions', 'amo_key')
         secret = config.get('extensions', 'amo_secret')
 
         url = ('https://addons.mozilla.org/api/v3/addons/{}/'
                'versions/{}/').format(app_id, version)
 
-        request = self.generate_jwt_request(iss, secret, url, 'GET')
+        request = self.generate_mozilla_jwt_request(
+            iss, secret, url, 'GET',
+        )
         response = json.load(urllib2.urlopen(request))
 
         filename = '{}-{}.xpi'.format(self.basename, version)
         self.path = os.path.join(
             config.get('extensions', 'nightliesDirectory'),
             self.basename,
-            filename
+            filename,
         )
 
         necessary = ['passed_review', 'reviewed', 'processed', 'valid']
         if all(response[x] for x in necessary):
             download_url = response['files'][0]['download_url']
             checksum = response['files'][0]['hash']
 
-            request = self.generate_jwt_request(iss, secret, download_url,
-                                                'GET')
+            request = self.generate_mozilla_jwt_request(
+                iss, secret, download_url, 'GET',
+            )
             try:
                 response = urllib2.urlopen(request)
             except urllib2.HTTPError as e:
                 logging.error(e.read())
 
             # Verify the extension's integrity
             file_content = response.read()
             sha256 = hashlib.sha256(file_content)
             returned_checksum = '{}:{}'.format(sha256.name, sha256.hexdigest())
 
             if returned_checksum != checksum:
                 logging.error('Checksum could not be verified: {} vs {}'
                               ''.format(checksum, returned_checksum))
 
             with open(self.path, 'w') as fp:
                 fp.write(file_content)
 
             self.update_link = os.path.join(
                 config.get('extensions', 'nightliesURL'),
                 self.basename,
-                filename
+                filename,
             )
 
             self.remove_from_downloads_lockfile(self.config.type,
                                                 'version',
                                                 version)
         elif not response['passed_review'] or not response['valid']:
             # When the review failed for any reason, we want to know about it
             logging.error(json.dumps(response, indent=4))
             self.remove_from_downloads_lockfile(self.config.type,
                                                 'version',
                                                 version)
 
     def uploadToChromeWebStore(self):
 
         opener = urllib2.build_opener(HTTPErrorBodyHandler)
 
         # use refresh token to obtain a valid access token
         # https://developers.google.com/accounts/docs/OAuth2WebServer#refresh
 
         response = json.load(opener.open(
             'https://accounts.google.com/o/oauth2/token',
 
             urlencode([
                 ('refresh_token', self.config.refreshToken),
                 ('client_id', self.config.clientID),
                 ('client_secret', self.config.clientSecret),
                 ('grant_type', 'refresh_token'),
-            ])
+            ]),
         ))
 
         auth_token = '%s %s' % (response['token_type'], response['access_token'])
 
         # upload a new version with the Chrome Web Store API
         # https://developer.chrome.com/webstore/using_webstore_api#uploadexisitng
 
         request = urllib2.Request('https://www.googleapis.com/upload/chromewebstore/v1.1/items/' + self.config.devbuildGalleryID)
         request.get_method = lambda: 'PUT'
         request.add_header('Authorization', auth_token)
@@ skipping 24 matching lines @@
         request.add_header('Content-Length', '0')
 
         response = json.load(opener.open(request))
 
         if any(status not in ('OK', 'ITEM_PENDING_REVIEW') for status in response['status']):
             raise Exception({'status': response['status'], 'statusDetail': response['statusDetail']})
 
     def generate_certificate_token_request(self, url, private_key):
         # Construct the token request according to
         # https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-certificate-credentials
-        def base64url_encode(data):
-            return base64.urlsafe_b64encode(data).replace(b'=', b'')

[Sebastian Noack, 2018/04/14 02:47:30]

How about .rstrip(b'=')?

[tlucas, 2018/04/14 08:55:46]

see below

[tlucas, 2018/04/16 14:50:09]

FWIW, there's no stripping / replacing done anymore (turns out it wasn't
necessary for our setup)

-
-        segments = []
-
         hex_val = binascii.a2b_hex(self.config.thumbprint)
         x5t = base64.urlsafe_b64encode(hex_val).decode()
 
-        now = datetime.datetime.now()

[Sebastian Noack, 2018/04/14 02:47:30]

It seems you are relying on the fact that our servers run with UTC timezone. It
would be better to request UTC time explicitly. Also you can create a timetuple
right away, without first creating a datetime object.

  now = int(time.mktime(time.gmtime()))
  expires = now + 600

[tlucas, 2018/04/14 08:55:46]

I'm actually thinking about refactoring parts of this in to
generate_jwt_request(), where no stripping (above comment) is done and which
already uses mktime().

[tlucas, 2018/04/16 14:50:09]

Done, almost: mktime() expects a time_struct in local_time format and produces
UTC-time out of it, so i'll use time.localtime() instead.

[Sebastian Noack, 2018/04/16 16:13:29]

You are right. But time.time() gives the same result.

[tlucas, 2018/04/16 16:50:24]

Done.

-        minutes = datetime.timedelta(0, 0, 0, 0, 10)
-        expires = now + minutes
-
-        # generate the full jwt body
-        jwt_payload = {
-            'aud': url,
-            'iss': self.config.clientID,
-            'sub': self.config.clientID,
-            'nbf': int(time.mktime(now.timetuple())),
-            'exp': int(time.mktime(expires.timetuple())),
-            'jti': str(uuid.uuid4()),
-        }
-
-        jwt_headers = {'typ': 'JWT', 'alg': 'RS256', 'x5t': x5t}
-
-        # sign the jwt body with the given private key
         key = RSA.importKey(private_key)
 
-        segments.append(base64url_encode(json.dumps(jwt_headers)))
-        segments.append(base64url_encode(json.dumps(jwt_payload)))

[Sebastian Noack, 2018/04/14 02:47:30]

Since we don't append to segments above, how about defining the list here?

  segments = [base64url_encode(json.dumps(jwt_headers)),
              base64url_encode(json.dumps(jwt_payload))]

[tlucas, 2018/04/14 08:55:46]

Acknowledged.

[tlucas, 2018/04/16 14:50:09]

Done.

-
-        body = b'.'.join(segments)
-        signature = PKCS1_v1_5.new(key).sign(Crypto.Hash.SHA256.new(body))
-
-        segments.append(base64url_encode(signature))
-        signed_jwt = b'.'.join(segments)
+        signed = self.sign_jwt(self.config.clientID, key, url,
+                               self.azure_jwt_signature_fnc,
+                               jwt_headers={'x5t': x5t})
 
         # generate oauth parameters for login.microsoft.com
         oauth_params = {
             'grant_type': 'client_credentials',
             'client_id': self.config.clientID,
             'resource': 'https://graph.windows.net',
             'client_assertion_type': 'urn:ietf:params:oauth:client-assertion-'
                                      'type:jwt-bearer',
-            'client_assertion': signed_jwt,
+            'client_assertion': signed,
         }
 
         request = urllib2.Request(url, urlencode(oauth_params))
         request.get_method = lambda: 'POST'
 
         return request
 
     def get_windows_store_access_token(self):
         # use client certificate to obtain a valid access token
-        url = 'https://login.microsoftonline.com/{}/oauth2/token'.format(
-            self.config.tenantID
-        )
+        url_template = 'https://login.microsoftonline.com/{}/oauth2/token'
+        url = url_template.format(self.config.tenantID)
 
         with open(self.config.privateKey, 'r') as fp:
             private_key = fp.read()
 
         opener = urllib2.build_opener(HTTPErrorBodyHandler)
         request = self.generate_certificate_token_request(url, private_key)
 
         with contextlib.closing(opener.open(request)) as response:
             data = json.load(response)
             auth_token = '{0[token_type]} {0[access_token]}'.format(data)
@@ skipping 25 matching lines @@
     def upload_to_windows_store(self):
         opener = urllib2.build_opener(HTTPErrorBodyHandler)
 
         headers = {'Authorization': self.get_windows_store_access_token(),
                    'Content-type': 'application/json'}
 
         # Get application
         # https://docs.microsoft.com/en-us/windows/uwp/monetize/get-an-app
         api_path = '{}/v1.0/my/applications/{}'.format(
             'https://manage.devcenter.microsoft.com',
-            self.config.devbuildGalleryID
+            self.config.devbuildGalleryID,
         )
 
         request = urllib2.Request(api_path, None, headers)
         with contextlib.closing(opener.open(request)) as response:
             app_obj = json.load(response)
 
         # Delete existing in-progress submission
         # https://docs.microsoft.com/en-us/windows/uwp/monetize/delete-an-app-submission
         submissions_path = api_path + '/submissions'
         if 'pendingApplicationSubmission' in app_obj:
@@ skipping 126 matching lines @@
                         # write update manifest
                         self.writeUpdateManifest()
 
                         # retire old builds
                         versions = self.retireBuilds()
                         # update index page
                         self.updateIndex(versions)
 
                         # Update soft link to latest build
                         baseDir = os.path.join(
-                            self.config.nightliesDirectory, self.basename
+                            self.config.nightliesDirectory, self.basename,
                         )
                         linkPath = os.path.join(
-                            baseDir, '00latest' + self.config.packageSuffix
+                            baseDir, '00latest' + self.config.packageSuffix,
                         )
 
                         self.symlink_or_copy(self.path, linkPath)
             finally:
                 # clean up
                 if self.tempdir:
                     shutil.rmtree(self.tempdir, ignore_errors=True)
 
 
 def main(download=False):
@@ skipping 23 matching lines @@
 
     file = open(nightlyConfigFile, 'wb')
     nightlyConfig.write(file)
 
 
 if __name__ == '__main__':
     parser = argparse.ArgumentParser()
     parser.add_argument('--download', action='store_true', default=False)
     args = parser.parse_args()
     main(args.download)