Fix root certificate issues for add-on store (#18354)

seanbudd · web-flow · commit b15c62947e77 · 2025-07-02T13:09:19.000+10:00
Summary of the issue:

If a device doesn't trust NV Access's TLS certificate, fetches to the add-on store fails.
When performing an update check, we update windows root certificates if our certificate is invalid or out of date.
Description of user facing changes:

Users without a trusted NV Access TLS certificate installed already should be able to access the add-on store
Description of developer facing changes:

Code related to updating root certificates is deprecated as it is not intended for the public
Description of development approach:

Made the code to update certificates more generic
diff --git a/source/addonStore/dataManager.py b/source/addonStore/dataManager.py
@@ -27,6 +27,7 @@
 from logHandler import log
 import NVDAState
 from NVDAState import WritePaths
+from utils.networking import _fetchUrlAndUpdateRootCertificates
 
 from .models.addon import (
 	AddonStoreModel,
@@ -135,11 +136,11 @@ def _getLatestAddonsDataForVersion(self, apiVersion: str) -> Optional[bytes]:
 			return None
 		return response.content
 
-	def _getCacheHash(self) -> Optional[str]:
+	def _getCacheHash(self) -> str | None:
 		url = _getCacheHashURL()
 		try:
 			log.debug(f"Fetching add-on data from {url}")
-			response = requests.get(url, timeout=FETCH_TIMEOUT_S)
+			response = _fetchUrlAndUpdateRootCertificates(url)
 		except requests.exceptions.RequestException as e:
 			log.debugWarning(f"Unable to get cache hash: {e}")
 			return None
diff --git a/source/updateCheck.py b/source/updateCheck.py
@@ -48,9 +48,6 @@
 import urllib.request
 import urllib.parse
 import hashlib
-import ctypes.wintypes
-import requests
-import ssl
 import wx
 import languageHandler
 
@@ -68,9 +65,41 @@
 import addonAPIVersion
 from logHandler import log, isPathExternalToNVDA
 import winKernel
+from utils.networking import _fetchUrlAndUpdateRootCertificates
 from utils.tempFile import _createEmptyTempFileForDeletingFile
 from dataclasses import dataclass
 
+import NVDAState
+
+
+def __getattr__(attrName: str) -> Any:
+	"""Module level `__getattr__` used to preserve backward compatibility."""
+	if attrName == "CERT_USAGE_MATCH" and NVDAState._allowDeprecatedAPI():
+		log.warning(
+			"CERT_USAGE_MATCH is deprecated and will be removed in a future version of NVDA. ",
+			stack_info=True,
+		)
+		from utils.networking import _CERT_USAGE_MATCH as CERT_USAGE_MATCH
+
+		return CERT_USAGE_MATCH
+	if attrName == "CERT_CHAIN_PARA" and NVDAState._allowDeprecatedAPI():
+		log.warning(
+			"CERT_CHAIN_PARA is deprecated and will be removed in a future version of NVDA. ",
+			stack_info=True,
+		)
+		from utils.networking import _CERT_CHAIN_PARA as CERT_CHAIN_PARA
+
+		return CERT_CHAIN_PARA
+	if attrName == "UPDATE_FETCH_TIMEOUT_S" and NVDAState._allowDeprecatedAPI():
+		log.warning(
+			"UPDATE_FETCH_TIMEOUT_S is deprecated and will be removed in a future version of NVDA. ",
+			stack_info=True,
+		)
+		from utils.networking import _FETCH_TIMEOUT_S as UPDATE_FETCH_TIMEOUT_S
+
+		return UPDATE_FETCH_TIMEOUT_S
+	raise AttributeError(f"module {repr(__name__)} has no attribute {repr(attrName)}")
+
 
 #: The URL to use for update checks.
 _DEFAULT_CHECK_URL = "https://api.nvaccess.org/nvdaUpdateCheck"
@@ -176,9 +205,6 @@ def getQualifiedDriverClassNameForStats(cls):
 	return "%s (core)" % name
 
 
-UPDATE_FETCH_TIMEOUT_S = 30  # seconds
-
-
 def checkForUpdate(auto: bool = False) -> UpdateInfo | None:
 	"""Check for an updated version of NVDA.
 	This will block, so it generally shouldn't be called from the main thread.
@@ -230,28 +256,17 @@ def checkForUpdate(auto: bool = False) -> UpdateInfo | None:
 		}
 		params.update(extraParams)
 
-	url = f"{_getCheckURL()}?{urllib.parse.urlencode(params)}"
-	try:
-		log.debug(f"Fetching update data from {url}")
-		res = urllib.request.urlopen(url, timeout=UPDATE_FETCH_TIMEOUT_S)
-	except IOError as e:
-		if (
-			isinstance(e.reason, ssl.SSLCertVerificationError)
-			and e.reason.reason == "CERTIFICATE_VERIFY_FAILED"
-		):
-			# #4803: Windows fetches trusted root certificates on demand.
-			# Python doesn't trigger this fetch (PythonIssue:20916), so try it ourselves
-			_updateWindowsRootCertificates()
-			# Retry the update check
-			log.debug(f"Retrying update check from {url}")
-			res = urllib.request.urlopen(url, timeout=UPDATE_FETCH_TIMEOUT_S)
-		else:
-			raise
+	result = _fetchUrlAndUpdateRootCertificates(
+		url=f"{_getCheckURL()}?{urllib.parse.urlencode(params)}",
+		# We must specify versionType so the server doesn't return a 404 error and
+		# thus cause an exception.
+		certFetchUrl=f"{_getCheckURL()}?versionType=stable",
+	)
 
-	if res.code != 200:
-		raise RuntimeError(f"Checking for update failed with HTTP status code {res.code}.")
+	if result.status_code != 200:
+		raise RuntimeError(f"Checking for update failed with HTTP status code {result.status_code}.")
 
-	data = res.read().decode("utf-8")  # Ensure the response is decoded correctly
+	data = result.content.decode("utf-8")  # Ensure the response is decoded correctly
 	# if data is empty, we return None, because the server returns an empty response if there is no update.
 	if not data:
 		return None
@@ -1038,68 +1053,3 @@ def terminate():
 	if autoChecker:
 		autoChecker.terminate()
 		autoChecker = None
-
-
-# These structs are only complete enough to achieve what we need.
-class CERT_USAGE_MATCH(ctypes.Structure):
-	_fields_ = (
-		("dwType", ctypes.wintypes.DWORD),
-		# CERT_ENHKEY_USAGE struct
-		("cUsageIdentifier", ctypes.wintypes.DWORD),
-		("rgpszUsageIdentifier", ctypes.c_void_p),  # LPSTR *
-	)
-
-
-class CERT_CHAIN_PARA(ctypes.Structure):
-	_fields_ = (
-		("cbSize", ctypes.wintypes.DWORD),
-		("RequestedUsage", CERT_USAGE_MATCH),
-		("RequestedIssuancePolicy", CERT_USAGE_MATCH),
-		("dwUrlRetrievalTimeout", ctypes.wintypes.DWORD),
-		("fCheckRevocationFreshnessTime", ctypes.wintypes.BOOL),
-		("dwRevocationFreshnessTime", ctypes.wintypes.DWORD),
-		("pftCacheResync", ctypes.c_void_p),  # LPFILETIME
-		("pStrongSignPara", ctypes.c_void_p),  # PCCERT_STRONG_SIGN_PARA
-		("dwStrongSignFlags", ctypes.wintypes.DWORD),
-	)
-
-
-def _updateWindowsRootCertificates():
-	log.debug("Updating Windows root certificates")
-	crypt = ctypes.windll.crypt32
-	with requests.get(
-		# We must specify versionType so the server doesn't return a 404 error and
-		# thus cause an exception.
-		f"{_getCheckURL()}?versionType=stable",
-		timeout=UPDATE_FETCH_TIMEOUT_S,
-		# Use an unverified connection to avoid a certificate error.
-		verify=False,
-		stream=True,
-	) as response:
-		# Get the server certificate.
-		cert = response.raw.connection.sock.getpeercert(True)
-	# Convert to a form usable by Windows.
-	certCont = crypt.CertCreateCertificateContext(
-		0x00000001,  # X509_ASN_ENCODING
-		cert,
-		len(cert),
-	)
-	# Ask Windows to build a certificate chain, thus triggering a root certificate update.
-	chainCont = ctypes.c_void_p()
-	crypt.CertGetCertificateChain(
-		None,
-		certCont,
-		None,
-		None,
-		ctypes.byref(
-			CERT_CHAIN_PARA(
-				cbSize=ctypes.sizeof(CERT_CHAIN_PARA),
-				RequestedUsage=CERT_USAGE_MATCH(),
-			),
-		),
-		0,
-		None,
-		ctypes.byref(chainCont),
-	)
-	crypt.CertFreeCertificateChain(chainCont)
-	crypt.CertFreeCertificateContext(certCont)
diff --git a/source/utils/networking.py b/source/utils/networking.py
@@ -0,0 +1,114 @@
+# A part of NonVisual Desktop Access (NVDA)
+# This file is covered by the GNU General Public License.
+# See the file COPYING for more details.
+# Copyright (C) 2012-2025 NV Access Limited, Zahari Yurukov,
+# Babbage B.V., Joseph Lee, Christopher Proß
+
+import ctypes
+import ctypes.wintypes
+import ssl
+
+import requests
+
+from logHandler import log
+
+
+_FETCH_TIMEOUT_S = 30
+"""Timeout for fetching in seconds."""
+
+
+# These structs are only complete enough to achieve what we need.
+class _CERT_USAGE_MATCH(ctypes.Structure):
+	_fields_ = (
+		("dwType", ctypes.wintypes.DWORD),
+		# CERT_ENHKEY_USAGE struct
+		("cUsageIdentifier", ctypes.wintypes.DWORD),
+		("rgpszUsageIdentifier", ctypes.c_void_p),  # LPSTR *
+	)
+
+
+class _CERT_CHAIN_PARA(ctypes.Structure):
+	_fields_ = (
+		("cbSize", ctypes.wintypes.DWORD),
+		("RequestedUsage", _CERT_USAGE_MATCH),
+		("RequestedIssuancePolicy", _CERT_USAGE_MATCH),
+		("dwUrlRetrievalTimeout", ctypes.wintypes.DWORD),
+		("fCheckRevocationFreshnessTime", ctypes.wintypes.BOOL),
+		("dwRevocationFreshnessTime", ctypes.wintypes.DWORD),
+		("pftCacheResync", ctypes.c_void_p),  # LPFILETIME
+		("pStrongSignPara", ctypes.c_void_p),  # PCCERT_STRONG_SIGN_PARA
+		("dwStrongSignFlags", ctypes.wintypes.DWORD),
+	)
+
+
+def _updateWindowsRootCertificates(url: str) -> None:
+	"""Updates the Windows root certificates by fetching the latest certificate from the server."""
+	log.debug("Updating Windows root certificates")
+	crypt = ctypes.windll.crypt32
+	with requests.get(
+		url,
+		timeout=_FETCH_TIMEOUT_S,
+		# Use an unverified connection to avoid a certificate error.
+		verify=False,
+		stream=True,
+	) as response:
+		# Get the server certificate.
+		cert = response.raw.connection.sock.getpeercert(True)
+	# Convert to a form usable by Windows.
+	certCont = crypt.CertCreateCertificateContext(
+		0x00000001,  # X509_ASN_ENCODING
+		cert,
+		len(cert),
+	)
+	# Ask Windows to build a certificate chain, thus triggering a root certificate update.
+	chainCont = ctypes.c_void_p()
+	crypt.CertGetCertificateChain(
+		None,
+		certCont,
+		None,
+		None,
+		ctypes.byref(
+			_CERT_CHAIN_PARA(
+				cbSize=ctypes.sizeof(_CERT_CHAIN_PARA),
+				RequestedUsage=_CERT_USAGE_MATCH(),
+			),
+		),
+		0,
+		None,
+		ctypes.byref(chainCont),
+	)
+	crypt.CertFreeCertificateChain(chainCont)
+	crypt.CertFreeCertificateContext(certCont)
+
+
+def _is_cert_verification_error(exception: requests.exceptions.SSLError) -> bool:
+	return (
+		exception.__context__
+		and exception.__context__.__cause__
+		and exception.__context__.__cause__.__context__
+		and isinstance(exception.__context__.__cause__.__context__, ssl.SSLCertVerificationError)
+		and exception.__context__.__cause__.__context__.reason == "CERTIFICATE_VERIFY_FAILED"
+	)
+
+
+def _fetchUrlAndUpdateRootCertificates(url: str, certFetchUrl: str | None = None) -> requests.Response:
+	"""Fetches the content of a URL and updates the Windows root certificates.
+
+	:param url: The URL to fetch.
+	:param certFetchUrl: An optional URL to use for fetching the certificate if the original URL fails due to a certificate error.
+	:return: The content of the URL.
+	"""
+	try:
+		log.debug(f"Fetching data from: {url}")
+		result = requests.get(url, timeout=_FETCH_TIMEOUT_S)
+		log.debug(f"Got response with status code: {result.status_code}")
+	except requests.exceptions.SSLError as e:
+		if _is_cert_verification_error(e):
+			# #4803: Windows fetches trusted root certificates on demand.
+			# Python doesn't trigger this fetch (PythonIssue:20916), so try it ourselves.
+			_updateWindowsRootCertificates(certFetchUrl or url)
+			log.debug(f"Retrying fetching data from: {url}")
+			result = requests.get(url, timeout=_FETCH_TIMEOUT_S)
+		else:
+			raise
+	return result
diff --git a/user_docs/en/changes.md b/user_docs/en/changes.md
@@ -93,6 +93,10 @@ The several built-in table definitions are moved to the `__tables` module in tha
 * Microsoft SQL Server Management Studio now uses the Visual Studio app module, as SSMS is based on Visual Studio. (#18176, @LeonarddeR)
 * NVDA will report Windows release revision number (for example: 10.0.26100.0) when `winVersion.getWinVer` is called and log this information at startup. (#18266, @josephsl)
 
+#### Deprecations
+
+* The following symbols in `updateCheck` are deprecated for removal without replacement: `CERT_USAGE_MATCH`, `CERT_CHAIN_PARA`, `UPDATE_FETCH_TIMEOUT_S`. (#18354)
+
 ## 2025.1.2
 
 This is a patch release to fix a bug.
