certs: fix interpolation and disallow relative paths (#44030)

5 files changed, 60 insertions, 64 deletions

diff --git a/lib/spack/docs/config_yaml.rst b/lib/spack/docs/config_yaml.rst
index f3f3833d5f..92ae421571 100644
--- a/lib/spack/docs/config_yaml.rst
+++ b/lib/spack/docs/config_yaml.rst
@@ -150,7 +150,7 @@ this can expose you to attacks.  Use at your own risk.
 --------------------
 
 Path to custom certificats for SSL verification. The value can be a 
-filesytem path, or an environment variable that expands to a file path.
+filesytem path, or an environment variable that expands to an absolute file path.
 The default value is set to the environment variable ``SSL_CERT_FILE``
 to use the same syntax used by many other applications that automatically
 detect custom certificates.
@@ -160,6 +160,9 @@ in the subprocess calling ``curl``.
 If ``url_fetch_method:urllib`` then files and directories are supported i.e. 
 ``config:ssl_certs:$SSL_CERT_FILE`` or ``config:ssl_certs:$SSL_CERT_DIR``
 will work.
+In all cases the expanded path must be absolute for Spack to use the certificates.
+Certificates relative to an environment can be created by prepending the path variable
+with the Spack configuration variable``$env``.
 
 --------------------
 ``checksum``
diff --git a/lib/spack/spack/oci/opener.py b/lib/spack/spack/oci/opener.py
index 182e60eb94..0717101274 100644
--- a/lib/spack/spack/oci/opener.py
+++ b/lib/spack/spack/oci/opener.py
@@ -398,7 +398,7 @@ def create_opener():
     opener = urllib.request.OpenerDirector()
     for handler in [
         urllib.request.UnknownHandler(),
-        urllib.request.HTTPSHandler(),
+        urllib.request.HTTPSHandler(context=spack.util.web.ssl_create_default_context()),
         spack.util.web.SpackHTTPDefaultErrorHandler(),
         urllib.request.HTTPRedirectHandler(),
         urllib.request.HTTPErrorProcessor(),
diff --git a/lib/spack/spack/reporters/cdash.py b/lib/spack/spack/reporters/cdash.py
index e6e4c07ea9..502f89d764 100644
--- a/lib/spack/spack/reporters/cdash.py
+++ b/lib/spack/spack/reporters/cdash.py
@@ -27,7 +27,7 @@ import spack.util.git
 from spack.error import SpackError
 from spack.util.crypto import checksum
 from spack.util.log_parse import parse_log_events
-from spack.util.web import urllib_ssl_cert_handler
+from spack.util.web import ssl_create_default_context
 
 from .base import Reporter
 from .extract import extract_test_parts
@@ -428,7 +428,7 @@ class CDash(Reporter):
         # Compute md5 checksum for the contents of this file.
         md5sum = checksum(hashlib.md5, filename, block_size=8192)
 
-        opener = build_opener(HTTPSHandler(context=urllib_ssl_cert_handler()))
+        opener = build_opener(HTTPSHandler(context=ssl_create_default_context()))
         with open(filename, "rb") as f:
             params_dict = {
                 "build": self.buildname,
diff --git a/lib/spack/spack/test/web.py b/lib/spack/spack/test/web.py
index 0b6d03e0bc..a2b64798d0 100644
--- a/lib/spack/spack/test/web.py
+++ b/lib/spack/spack/test/web.py
@@ -415,7 +415,7 @@ def test_ssl_urllib(
 
         assert mock_cert == spack.config.get("config:ssl_certs", None)
 
-        ssl_context = spack.util.web.urllib_ssl_cert_handler()
+        ssl_context = spack.util.web.ssl_create_default_context()
         assert ssl_context.verify_mode == ssl.CERT_REQUIRED
 
 
diff --git a/lib/spack/spack/util/web.py b/lib/spack/spack/util/web.py
index 8da447b8bf..8c843c5346 100644
--- a/lib/spack/spack/util/web.py
+++ b/lib/spack/spack/util/web.py
@@ -12,12 +12,13 @@ import os.path
 import re
 import shutil
 import ssl
+import stat
 import sys
 import traceback
 import urllib.parse
 from html.parser import HTMLParser
 from pathlib import Path, PurePosixPath
-from typing import IO, Dict, Iterable, List, Optional, Set, Union
+from typing import IO, Dict, Iterable, List, Optional, Set, Tuple, Union
 from urllib.error import HTTPError, URLError
 from urllib.request import HTTPSHandler, Request, build_opener
 
@@ -30,7 +31,7 @@ import spack.error
 import spack.util.path
 import spack.util.url as url_util
 
-from .executable import CommandNotFoundError, which
+from .executable import CommandNotFoundError, Executable, which
 from .gcs import GCSBlob, GCSBucket, GCSHandler
 from .s3 import UrllibS3Handler, get_s3_session
 
@@ -60,64 +61,56 @@ class SpackHTTPDefaultErrorHandler(urllib.request.HTTPDefaultErrorHandler):
         raise DetailedHTTPError(req, code, msg, hdrs, fp)
 
 
-dbg_msg_no_ssl_cert_config = (
-    "config:ssl_certs not in configuration. "
-    "Default cert configuation and environment will be used."
-)
-
-
-def urllib_ssl_cert_handler():
-    """context for configuring ssl during urllib HTTPS operations"""
-    custom_cert_var = spack.config.get("config:ssl_certs")
-    if custom_cert_var:
-        # custom certs will be a location, so expand env variables, paths etc
-        certs = spack.util.path.canonicalize_path(custom_cert_var)
-        tty.debug("URLLIB: Looking for custom SSL certs at {}".format(certs))
-        if os.path.isfile(certs):
-            tty.debug("URLLIB: Custom SSL certs file found at {}".format(certs))
-            return ssl.create_default_context(cafile=certs)
-        elif os.path.isdir(certs):
-            tty.debug("URLLIB: Custom SSL certs directory found at {}".format(certs))
-            return ssl.create_default_context(capath=certs)
-        else:
-            tty.debug("URLLIB: Custom SSL certs not found")
-            return ssl.create_default_context()
-    else:
-        tty.debug(dbg_msg_no_ssl_cert_config)
-        return ssl.create_default_context()
+def custom_ssl_certs() -> Optional[Tuple[bool, str]]:
+    """Returns a tuple (is_file, path) if custom SSL certifates are configured and valid."""
+    ssl_certs = spack.config.get("config:ssl_certs")
+    if not ssl_certs:
+        return None
+    path = spack.util.path.substitute_path_variables(ssl_certs)
+    if not os.path.isabs(path):
+        tty.debug(f"certs: relative path not allowed: {path}")
+        return None
+    try:
+        st = os.stat(path)
+    except OSError as e:
+        tty.debug(f"certs: error checking path {path}: {e}")
+        return None
 
+    file_type = stat.S_IFMT(st.st_mode)
 
-# curl requires different strategies for custom certs at runtime depending on if certs
-# are stored as a file or a directory
-def append_curl_env_for_ssl_certs(curl):
-    """
-    configure curl to use custom certs in a file at run time
-    see: https://curl.se/docs/sslcerts.html item 4
-    """
-    custom_cert_var = spack.config.get("config:ssl_certs")
-    if custom_cert_var:
-        # custom certs will be a location, so expand env variables, paths etc
-        certs = spack.util.path.canonicalize_path(custom_cert_var)
-        tty.debug("CURL: Looking for custom SSL certs file at {}".format(certs))
-        if os.path.isfile(certs):
-            tty.debug(
-                "CURL: Configuring curl to use custom"
-                " certs from {} by setting "
-                "CURL_CA_BUNDLE".format(certs)
-            )
-            curl.add_default_env("CURL_CA_BUNDLE", certs)
-        elif os.path.isdir(certs):
-            tty.warn(
-                "CURL config:ssl_certs"
-                " is a directory but cURL only supports files. Default certs will be used instead."
-            )
-        else:
-            tty.debug(
-                "CURL config:ssl_certs "
-                "resolves to {}. This is not a file so default certs will be used.".format(certs)
-            )
+    if file_type != stat.S_IFREG and file_type != stat.S_IFDIR:
+        tty.debug(f"certs: not a file or directory: {path}")
+        return None
+
+    return (file_type == stat.S_IFREG, path)
+
+
+def ssl_create_default_context():
+    """Create the default SSL context for urllib with custom certificates if configured."""
+    certs = custom_ssl_certs()
+    if certs is None:
+        return ssl.create_default_context()
+    is_file, path = certs
+    if is_file:
+        tty.debug(f"urllib: certs: using cafile {path}")
+        return ssl.create_default_context(cafile=path)
     else:
-        tty.debug(dbg_msg_no_ssl_cert_config)
+        tty.debug(f"urllib: certs: using capath {path}")
+        return ssl.create_default_context(capath=path)
+
+
+def set_curl_env_for_ssl_certs(curl: Executable) -> None:
+    """configure curl to use custom certs in a file at runtime. See:
+    https://curl.se/docs/sslcerts.html item 4"""
+    certs = custom_ssl_certs()
+    if certs is None:
+        return
+    is_file, path = certs
+    if not is_file:
+        tty.debug(f"curl: {path} is not a file: default certs will be used.")
+        return
+    tty.debug(f"curl: using CURL_CA_BUNDLE={path}")
+    curl.add_default_env("CURL_CA_BUNDLE", path)
 
 
 def _urlopen():
@@ -127,7 +120,7 @@ def _urlopen():
 
     # One opener with HTTPS ssl enabled
     with_ssl = build_opener(
-        s3, gcs, HTTPSHandler(context=urllib_ssl_cert_handler()), error_handler
+        s3, gcs, HTTPSHandler(context=ssl_create_default_context()), error_handler
     )
 
     # One opener with HTTPS ssl disabled
@@ -348,7 +341,7 @@ def _curl(curl=None):
         except CommandNotFoundError as exc:
             tty.error(str(exc))
             raise spack.error.FetchError("Missing required curl fetch method")
-    append_curl_env_for_ssl_certs(curl)
+    set_curl_env_for_ssl_certs(curl)
     return curl