Skip to content

Commit

Permalink
Fix OIDC access to verify TLS connection (#3487)
Browse files Browse the repository at this point in the history
* Add exception message to ServerConnectionError
* Abort keycloak retries on connection and other non-HTTP errors
* Use consistent approach to TLS verification for OIDC connection
  • Loading branch information
webbnh authored Jul 10, 2023
1 parent aaf890c commit 30ddc0b
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 18 deletions.
23 changes: 13 additions & 10 deletions lib/pbench/server/auth/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,10 @@ def wait_for_oidc_server(
try:
oidc_server = server_config.get("openid", "server_url")
oidc_realm = server_config.get("openid", "realm")
# Get a custom cert location to verify Keycloak ssl if its define
# in the config file. Otherwise, we default to using system-wide
# certificates.
cert = server_config.get("openid", "cert_location", fallback=True)
# Look for a custom CA to use in the verification of the TLS
# connection to the OIDC server. If it is undefined, the value of
# True will result in using the default TLS verification.
ca_cert = server_config.get("openid", "tls_ca_file", fallback=True)
except (NoOptionError, NoSectionError) as exc:
raise OpenIDClient.NotConfigured() from exc

Expand All @@ -224,6 +224,8 @@ def wait_for_oidc_server(
# https://urllib3.readthedocs.io/en/latest/reference/urllib3.util.html#module-urllib3.util.retry
retry = Retry(
total=8,
connect=0,
other=0,
backoff_factor=2,
status_forcelist=tuple(int(x) for x in HTTPStatus if x != 200),
)
Expand All @@ -236,11 +238,11 @@ def wait_for_oidc_server(
try:
response = session.get(
f"{oidc_server}/realms/{oidc_realm}/.well-known/openid-configuration",
verify=cert,
verify=ca_cert,
)
response.raise_for_status()
except Exception as exc:
raise OpenIDClient.ServerConnectionError() from exc
raise OpenIDClient.ServerConnectionError(str(exc)) from exc
if response.json().get("issuer") == f"{oidc_server}/realms/{oidc_realm}":
logger.debug("OIDC server connection verified")
connected = True
Expand Down Expand Up @@ -268,14 +270,15 @@ def construct_oidc_client(cls, server_config: PbenchServerConfig) -> "OpenIDClie
server_url = server_config.get("openid", "server_url")
client = server_config.get("openid", "client")
realm = server_config.get("openid", "realm")
# Look for a custom CA to use in the verification of the TLS
# connection to the OIDC server. If it is undefined, the value of
# True will result in using the default TLS verification.
ca_cert = server_config.get("openid", "tls_ca_file", fallback=True)
except (NoOptionError, NoSectionError) as exc:
raise OpenIDClient.NotConfigured() from exc

oidc_client = cls(
server_url=server_url,
client_id=client,
realm_name=realm,
verify=False,
server_url=server_url, client_id=client, realm_name=realm, verify=ca_cert
)
oidc_client.set_oidc_public_key()
return oidc_client
Expand Down
4 changes: 2 additions & 2 deletions lib/pbench/test/unit/server/auth/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ def test_wait_for_oidc_server_fail(self, make_logger):
config["openid"] = {
"server_url": "https://example.com",
"realm": "realm",
"cert_location": "/ca.crt",
"tls_ca_file": "/ca.crt",
}

# Keycloak well-known endpoint without any response
Expand Down Expand Up @@ -338,7 +338,7 @@ def test_wait_for_oidc_server_succ(self, make_logger):
config["openid"] = {
"server_url": "https://example.com",
"realm": "realm",
"cert_location": "/ca.crt",
"tls_ca_file": "/ca.crt",
}

# Keycloak well-known endpoint returning response with valid issuer
Expand Down
6 changes: 3 additions & 3 deletions server/lib/config/pbench-server-default.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@ realm = pbench-server
# Client entity name requesting OIDC to authenticate a user.
client = pbench-client

# Cert location for connecting to the OIDC client
# If you want to use a custom CA then its location path should be recorded.
#cert_location = /path/CA
# Custom CA for verifying the TLS connection to the OIDC client.
# If omitted, TLS verification will use the system's trusted CA list.
#tls_ca_file = /path/to/CA/file

[logging]
logger_type = devlog
Expand Down
5 changes: 2 additions & 3 deletions server/pbenchinacan/etc/pbench-server/pbench-server.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,8 @@ secret-key = "pbench-in-a-can secret shhh"
[openid]
server_url = https://localhost:8090

# Override the default cert value to use for pbenchinacan Keycloak container
# connection.
cert_location = /etc/pki/tls/certs/pbench_CA.crt
# Provide a CA cert for the pbenchinacan Keycloak server connection.
tls_ca_file = /etc/pki/tls/certs/pbench_CA.crt

###########################################################################
# The rest will come from the default config file.
Expand Down

0 comments on commit 30ddc0b

Please sign in to comment.