From d22bdc1c4e666690e81e67752dfa6831609a4845 Mon Sep 17 00:00:00 2001 From: psakievich Date: Tue, 7 May 2024 03:16:32 -0600 Subject: [PATCH] certs: fix interpolation and disallow relative paths (#44030) --- lib/spack/docs/config_yaml.rst | 5 +- lib/spack/spack/oci/opener.py | 2 +- lib/spack/spack/reporters/cdash.py | 4 +- lib/spack/spack/test/web.py | 2 +- lib/spack/spack/util/web.py | 109 ++++++++++++++--------------- 5 files changed, 59 insertions(+), 63 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 @@ 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 @@ def upload(self, filename): # 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 mock_verify_locations(self, cafile, capath, cadata): 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 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.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 @@ def http_error_default(self, req, fp, code, msg, hdrs): 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 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) + + 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 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) +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() - - -# 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) - ) + 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