net.download_file() only uses https:// by default

This strictly enforces things like this only use https://:

* `Binaries:` / `binary:` URLs
* `fdroid signatures` fetching APKs
* `fdroid install` fetching from GitHub

fdroidserver/net.py

@@ -36,7 +36,14 @@ logger = logging.getLogger(__name__)
 HEADERS = {'User-Agent': 'F-Droid'}
 
 
-def download_file(url, local_filename=None, dldir='tmp', retries=3, backoff_factor=0.1):
+def download_file(
+    url,
+    local_filename=None,
+    dldir='tmp',
+    retries=3,
+    backoff_factor=0.1,
+    https_only=True,
+):
     """Try hard to download the file, including retrying on failures.
 
     This has two retry cycles, one inside of the requests session, the
@@ -55,11 +62,16 @@ def download_file(url, local_filename=None, dldir='tmp', retries=3, backoff_fact
         if retries:
             max_retries = Retry(total=retries - i, backoff_factor=backoff_factor)
             adapter = HTTPAdapter(max_retries=max_retries)
-            session = requests.Session()
-            session.mount('http://', adapter)
-            session.mount('https://', adapter)
         else:
-            session = requests
+            adapter = HTTPAdapter()
+        session = requests.Session()
+        session.mount('https://', adapter)
+        if https_only:
+            for k in session.adapters:
+                if k != 'https://':
+                    del session.adapters[k]
+        else:
+            session.mount('http://', adapter)
         # the stream=True parameter keeps memory usage low
         r = session.get(
             url, stream=True, allow_redirects=True, headers=HEADERS, timeout=300

fdroidserver/signatures.py

@@ -68,7 +68,9 @@ def extract(options):
                         from . import net
 
                         tmp_apk = os.path.join(tmp_dir, 'signed.apk')
-                        net.download_file(apk, tmp_apk)
+                        net.download_file(
+                            apk, tmp_apk, https_only=not options.no_check_https
+                        )
                         sigdir = extract_signature(tmp_apk)
                         logging.info(
                             _(

tests/test_net.py

@@ -84,7 +84,7 @@ class NetTest(unittest.TestCase):
     def tearDown(self):
         self.tempdir.cleanup()
 
-    @patch('requests.get')
+    @patch('requests.Session.get')
     def test_download_file_url_parsing(self, requests_get):
         # pylint: disable=unused-argument
         def _get(url, stream, allow_redirects, headers, timeout):
@@ -104,10 +104,20 @@ class NetTest(unittest.TestCase):
         self.assertTrue(os.path.exists(f))
         self.assertEqual('tmp/com.downloader.aegis-3175421.apk', f)
 
+    @patch.dict(os.environ, clear=True)
+    def test_download_file_no_http(self):
+        with self.assertRaises(requests.exceptions.InvalidSchema):
+            net.download_file('http://neverssl.com/repo/entry.jar')
+
+    @patch.dict(os.environ, clear=True)
+    def test_download_file_no_git(self):
+        with self.assertRaises(requests.exceptions.InvalidSchema):
+            net.download_file('git://github.com/')
+
     @patch.dict(os.environ, clear=True)
     def test_download_file_retries(self):
         server = RetryServer()
-        f = net.download_file('http://localhost:%d/f.txt' % server.port)
+        f = net.download_file(f'http://localhost:{server.port}/f.txt', https_only=False)
         # strip the HTTP headers and compare the reply
         self.assertEqual(server.reply.split(b'\n\n')[1], Path(f).read_bytes())
         server.stop()
@@ -117,7 +127,7 @@ class NetTest(unittest.TestCase):
         """The retry logic should eventually exit with an error."""
         server = RetryServer(failures=5)
         with self.assertRaises(requests.exceptions.ConnectionError):
-            net.download_file('http://localhost:%d/f.txt' % server.port)
+            net.download_file(f'http://localhost:{server.port}/f.txt', https_only=False)
         server.stop()
 
     @unittest.skipIf(os.getenv('CI'), 'FIXME this fails mysteriously only in GitLab CI')