[AIRFLOW-2886] Secure Flask SECRET_KEY #3738
Conversation
|
hey @XD-DENG , it seems that it will still be an issue if we have a cluster for webservers based on the comment in https://issues.apache.org/jira/browse/AIRFLOW-2866. What do you think? cc @Fokko , @kaxil as they comment / approve the original prs. |
|
Hi @feng-tao , in cluster mode, given the I insist in that we should have random SECRET_KEY for the webserver. If we use the previous way, that is to have a default value in the template, it's gonna be risky. If users want to use a cluster of webservers, they need to either
Please let me know your thoughts? |
|
Are people using clusters of webservers? I've never seen such a setup to be honest. |
|
@Fokko @feng-tao : In a more common scenario, users have |
XD-DENG
force-pushed
the
patch-2
branch
3 times, most recently
from
12950af to
d6dc16d
Compare
The Flask SECRET_KEY should be as random as possible. On the other hand, we can nott genrate random value when we launch the webserver (the secret_key will be inconsistent across the workers). We can generate a random one in the configuration file airflow.cfg, just like how we deal with FERNET_KEY. The SECRET_KEY is generated using os.urandom, as recommended by Flask community.
Codecov Report
@@ Coverage Diff @@
## master #3738 +/- ##
==========================================
+ Coverage 77.66% 77.67% +0.01%
==========================================
Files 204 204
Lines 15849 15846 -3
==========================================
Hits 12309 12309
+ Misses 3540 3537 -3
Continue to review full report at Codecov.
|
|
looking at this #3651 pr, I am wondering whether we could do better than use random function to assign the value to secret_key. I felt the better approach is to have a random key store in an internal key management service(this should be common for company, e.g lyft uses https://github.com/lyft/confidant) and assign it to secret_key in the config file. This will make sure the key is random but uniform across all the webservers. This won't cause any csrf issues as well. What do you think @Fokko , @kaxil , @XD-DENG ? |
|
This pr currently can't address the issue when we have a cluster for webserver. IMO, I would prefer to revert the orignal prs and improve the secret_key config documentation. Let me know what you guys think. |
|
As far as I can tell, I can't save any passwords in Connections in Airflow webUI from a fresh master and this commit cherry-picked. I don't know if I could've caused this though. |
|
Hi @feng-tao , I agree that PRs #3651 and #3729 must be reverted as they are causing CSRF issues in web UI at this moment(they are also reverts in the commit of this PR). On the other hand, for Airflow, I don’t think it’s common to run multiple nodes for webserver for a single Airflow instance. Normally people use multiple processes on a single machine as multiple workers for each single Airflow instance. Then the solution in this PR can provide out-of-the-box security improvement. If you're talking about a cluster of Airflow instances, then Airflow instances do not need to have consistent secret_key, and they should not (different apps should use different secret_keys). Please let me know your thoughts. Thanks. |
|
My vote is to go with this approach (secure by default and easy for the common case) plus a bit of documentation saying that you will want to make sure that this value is the same across multiple machines if you run more than one behind a load balancer etc. |
|
To use an external secret store this could be extended to use the (already existing) |
|
sounds good @ashb . Let's go ahead to merge this pr to unblock master and document the functionality you mentioned later. |
|
thanks @XD-DENG for fixing the issue. |
The Flask SECRET_KEY should be as random as possible. On the other hand, we can nott genrate random value when we launch the webserver (the secret_key will be inconsistent across the workers). We can generate a random one in the configuration file airflow.cfg, just like how we deal with FERNET_KEY. The SECRET_KEY is generated using os.urandom, as recommended by Flask community.
The Flask SECRET_KEY should be as random as possible. On the other hand, we can nott genrate random value when we launch the webserver (the secret_key will be inconsistent across the workers). We can generate a random one in the configuration file airflow.cfg, just like how we deal with FERNET_KEY. The SECRET_KEY is generated using os.urandom, as recommended by Flask community.
) The Flask SECRET_KEY should be as random as possible. On the other hand, we can nott genrate random value when we launch the webserver (the secret_key will be inconsistent across the workers). We can generate a random one in the configuration file airflow.cfg, just like how we deal with FERNET_KEY. The SECRET_KEY is generated using os.urandom, as recommended by Flask community. (cherry picked from commit f7602f8)
) The Flask SECRET_KEY should be as random as possible. On the other hand, we can nott genrate random value when we launch the webserver (the secret_key will be inconsistent across the workers). We can generate a random one in the configuration file airflow.cfg, just like how we deal with FERNET_KEY. The SECRET_KEY is generated using os.urandom, as recommended by Flask community. (cherry picked from commit f7602f8)
) The Flask SECRET_KEY should be as random as possible. On the other hand, we can nott genrate random value when we launch the webserver (the secret_key will be inconsistent across the workers). We can generate a random one in the configuration file airflow.cfg, just like how we deal with FERNET_KEY. The SECRET_KEY is generated using os.urandom, as recommended by Flask community. (cherry picked from commit f7602f8)
…ache#3738) The Flask SECRET_KEY should be as random as possible. On the other hand, we can nott genrate random value when we launch the webserver (the secret_key will be inconsistent across the workers). We can generate a random one in the configuration file airflow.cfg, just like how we deal with FERNET_KEY. The SECRET_KEY is generated using os.urandom, as recommended by Flask community. (cherry picked from commit f7602f8) (cherry picked from commit 6b06584) (cherry picked from commit 18e9816b2961152972a41edde38ef648039a915f)
…ache#3738) The Flask SECRET_KEY should be as random as possible. On the other hand, we can nott genrate random value when we launch the webserver (the secret_key will be inconsistent across the workers). We can generate a random one in the configuration file airflow.cfg, just like how we deal with FERNET_KEY. The SECRET_KEY is generated using os.urandom, as recommended by Flask community. (cherry picked from commit f7602f8) (cherry picked from commit 6b06584) (cherry picked from commit 18e9816b2961152972a41edde38ef648039a915f) (cherry picked from commit cc91e3a)
…ache#3738) The Flask SECRET_KEY should be as random as possible. On the other hand, we can nott genrate random value when we launch the webserver (the secret_key will be inconsistent across the workers). We can generate a random one in the configuration file airflow.cfg, just like how we deal with FERNET_KEY. The SECRET_KEY is generated using os.urandom, as recommended by Flask community. (cherry picked from commit f7602f8) (cherry picked from commit 6b06584)
- BugFix: Tasks with ``depends_on_past`` or ``task_concurrency`` are stuck (apache#12663) - Fix issue with empty Resources in executor_config (apache#12633) - Fix: Deprecated config ``force_log_out_after`` was not used (apache#12661) - Fix empty asctime field in JSON formatted logs (apache#10515) - [AIRFLOW-2809] Fix security issue regarding Flask SECRET_KEY (apache#3651) - [AIRFLOW-2884] Fix Flask SECRET_KEY security issue in www_rbac (apache#3729) - [AIRFLOW-2886] Generate random Flask SECRET_KEY in default config (apache#3738) - Add missing comma in setup.py (apache#12790) - Bugfix: Unable to import Airflow plugins on Python 3.8 (apache#12859) - Fix setup.py missing comma in ``setup_requires`` (apache#12880) - Don't emit first_task_scheduling_delay metric for only-once dags (apache#12835) - Update setup.py to get non-conflicting set of dependencies (apache#12636) - Rename ``[scheduler] max_threads`` to ``[scheduler] parsing_processes`` (apache#12605) - Add metric for scheduling delay between first run task & expected start time (apache#9544) - Add new-style 2.0 command names for Airflow 1.10.x (apache#12725) - Add Kubernetes cleanup-pods CLI command for Helm Chart (apache#11802) - Don't let webserver run with dangerous config (apache#12747) - Replace pkg_resources with importlib.metadata to avoid VersionConflict errors (apache#12694) - Clarified information about supported Databases
The Flask SECRET_KEY should be as random as possible. On the other hand, we can nott genrate random value when we launch the webserver (the secret_key will be inconsistent across the workers). We can generate a random one in the configuration file airflow.cfg, just like how we deal with FERNET_KEY. The SECRET_KEY is generated using os.urandom, as recommended by Flask community. (cherry picked from commit f7602f8)
7 participants
Jira
Description
In my earlier PRs, #3651 and #3729 , I proposed to generate random
SECRET_KEYfor thewebserver(Flask App).However, I realise that we may encounter CSRF error
The CSRF session token is missingwhen we have multiple workers for the Flask webserver, since the secret_key is not consistent among workers.On the other hand, it's still very important to have as random SECRET_KEY as possible for security reasons. We can deal with it like how we dealt with
FERNET_KEY(i.e. generate a random value when the airflow.cfg file is initiated).Tests
Commits
Documentation
Code Quality
git diff upstream/master -u -- "*.py" | flake8 --diff