Skip to content

Commit

Permalink
fix: use custom TLS cert only for Determined API requests [DET-3360] (#…
Browse files Browse the repository at this point in the history
…716)

Previously, if the CLI found a custom trusted master certificate, it
would be used for all requests made using Requests; that was the
simplest thing to write, but it meant that other HTTPS requests would
fail (e.g., checkpoint downloads from GCS, which go through Requests
internally). This change restricts the cert to only be applied to
requests made to the master.
  • Loading branch information
dzhu authored Jun 15, 2020
1 parent f01c17c commit 6d8c07c
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 3 deletions.
4 changes: 2 additions & 2 deletions cli/determined_cli/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ def die(message: str, always_print_traceback: bool = False) -> None:

cert_fn = str(auth.get_config_path().joinpath("master.crt"))
if os.path.exists(cert_fn):
os.environ["REQUESTS_CA_BUNDLE"] = cert_fn
api.request.set_master_cert_bundle(cert_fn)

try:
try:
Expand Down Expand Up @@ -274,7 +274,7 @@ def die(message: str, always_print_traceback: bool = False) -> None:

with open(cert_fn, "w") as out:
out.write(cert_pem_data)
os.environ["REQUESTS_CA_BUNDLE"] = cert_fn
api.request.set_master_cert_bundle(cert_fn)

check_version(parsed_args)

Expand Down
17 changes: 16 additions & 1 deletion common/determined_common/api/request.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@

from determined_common.api import authentication, errors

# The path to a file containing an SSL certificate to trust specifically for the master, if any.
_master_cert_bundle = None


def set_master_cert_bundle(path: str) -> None:
global _master_cert_bundle
_master_cert_bundle = path


def parse_master_address(master_address: str) -> parse.ParseResult:
if master_address.startswith("https://"):
Expand Down Expand Up @@ -66,7 +74,14 @@ def do_request(
h = add_token_to_headers(h)

try:
r = requests.request(method, make_url(host, path), params=params, json=body, headers=h)
r = requests.request(
method,
make_url(host, path),
params=params,
json=body,
headers=h,
verify=_master_cert_bundle,
)
except requests.exceptions.SSLError:
raise
except requests.exceptions.ConnectionError as e:
Expand Down

0 comments on commit 6d8c07c

Please sign in to comment.