kaxil commented on code in PR #67214:
URL: https://github.com/apache/airflow/pull/67214#discussion_r3307010230
##########
airflow-core/src/airflow/api_fastapi/gunicorn_app.py:
##########
@@ -275,6 +276,8 @@ def create_gunicorn_app(
if ssl_cert and ssl_key:
options["certfile"] = ssl_cert
options["keyfile"] = ssl_key
+ if ssl_ca_file:
Review Comment:
The PR description says this is for mTLS to the execution API, but
gunicorn's `cert_reqs` defaults to `ssl.CERT_NONE`, so `ca_certs` is never
consulted for verifying client certificates. Setting just `ca_certs` here
doesn't enable mTLS -- it's a no-op until paired with `options["cert_reqs"] =
ssl.CERT_REQUIRED` (or `CERT_OPTIONAL`). Worth either adding that, or renaming
the option to reflect what it actually does on the server side.
##########
airflow-core/src/airflow/cli/commands/api_server_command.py:
##########
@@ -118,6 +119,7 @@ def _run_api_server_with_uvicorn(
"timeout_worker_healthcheck": worker_timeout,
"ssl_keyfile": ssl_key,
"ssl_certfile": ssl_cert,
+ "ssl_ca_certs": ssl_ca_file,
Review Comment:
Same issue as the gunicorn path: uvicorn's `ssl_cert_reqs` defaults to
`ssl.CERT_NONE`, so `ssl_ca_certs` doesn't gate anything. If the goal is mTLS,
also pass `ssl_cert_reqs=ssl.CERT_REQUIRED`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
