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 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
 )
 from sitescripts.utils import get_config, get_template
 
 MAX_BUILDS = 50
 
@@ skipping 285 matching lines @@
                 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 == 'safari':
+            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])
 
             if self.config.type not in {'gecko', 'edge'}:
                 command.extend(['-k', self.config.keyFile])
             command.append(self.path)
             subprocess.check_call(command, env=env)
 
@@ skipping 278 matching lines @@
         request.get_method = lambda: 'POST'
         request.add_header('Authorization', auth_token)
         request.add_header('x-goog-api-version', '2')
         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)
+
+        # 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,
+        }
+
+        request = urllib2.Request(url, urlencode(oauth_params))
+        request.get_method = lambda: 'POST'
+
+        return request
+
     def get_windows_store_access_token(self):
-        # use refresh token to obtain a valid access token
-        # https://docs.microsoft.com/en-us/azure/active-directory/active-directory-protocols-oauth-code#refreshing-the-access-tokens
-        server = 'https://login.microsoftonline.com'
-        token_path = '{}/{}/oauth2/token'.format(server, self.config.tenantID)
+        # use client certificate to obtain a valid access token
+        url = 'https://login.microsoftonline.com/{}/oauth2/token'.format(
+            self.config.tenantID
+        )
+
+        with open(self.config.privateKey, 'r') as fp:
+            private_key = fp.read()
 
         opener = urllib2.build_opener(HTTPErrorBodyHandler)
-        post_data = urlencode([
-            ('refresh_token', self.config.refreshToken),
-            ('client_id', self.config.clientID),
-            ('client_secret', self.config.clientSecret),
-            ('grant_type', 'refresh_token'),
-            ('resource', 'https://graph.windows.net')
-        ])
-        request = urllib2.Request(token_path, post_data)
+        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)
 
         return auth_token
 
     def upload_appx_file_to_windows_store(self, file_upload_url):
         # Add .appx file to a .zip file
         zip_path = os.path.splitext(self.path)[0] + '.zip'
         with zipfile.ZipFile(zip_path, 'w', zipfile.ZIP_DEFLATED) as zf:
@@ skipping 130 matching lines @@
 
             # update nightlies config
             self.config.latestRevision = self.revision
 
             if (self.config.type == 'gecko' and
                     self.config.galleryID and
                     get_config().has_option('extensions', 'amo_key')):
                 self.uploadToMozillaAddons()
             elif self.config.type == 'chrome' and self.config.clientID and self.config.clientSecret and self.config.refreshToken:
                 self.uploadToChromeWebStore()
-            elif self.config.type == 'edge' and self.config.clientID and self.config.clientSecret and self.config.refreshToken and self.config.tenantID:
+            elif self.config.type == 'edge' and self.config.clientID and self.config.tenantID and self.config.privateKey and self.config.thumbprint:
                 self.upload_to_windows_store()
 
         finally:
             # clean up
             if self.tempdir:
                 shutil.rmtree(self.tempdir, ignore_errors=True)
 
     def download(self):
         download_info = self.read_downloads_lockfile()
         downloads = self.downloadable_repos.intersection(download_info.keys())
@@ skipping 61 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)