[jira] [Commented] (AIRFLOW-2886) Secure Flask SECRET_KEY
[
https://issues.apache.org/jira/browse/AIRFLOW-2886?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16580293#comment-16580293
]
ASF subversion and git services commented on AIRFLOW-2886:
--
Commit f7602f8266559e55bc602a9639e3e1ab640f30e8 in incubator-airflow's branch
refs/heads/master from Xiaodong
[ https://gitbox.apache.org/repos/asf?p=incubator-airflow.git;h=f7602f8 ]
[AIRFLOW-2886] Secure Flask SECRET_KEY (#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.
> Secure Flask SECRET_KEY
> ---
>
> Key: AIRFLOW-2886
> URL: https://issues.apache.org/jira/browse/AIRFLOW-2886
> Project: Apache Airflow
> Issue Type: Bug
>Reporter: Xiaodong DENG
>Assignee: Xiaodong DENG
>Priority: Critical
>
> In my earlier PRs, [https://github.com/apache/incubator-airflow/pull/3651]
> and [https://github.com/apache/incubator-airflow/pull/3729] , I proposed to
> generate random SECRET_KEY for Flask App.
> If we have multiple workers for the Flask webserver, we may encounter CSRF
> error {{The CSRF session token is missing}} .
> 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).
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (AIRFLOW-2886) Secure Flask SECRET_KEY
[
https://issues.apache.org/jira/browse/AIRFLOW-2886?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16580292#comment-16580292
]
ASF GitHub Bot commented on AIRFLOW-2886:
-
feng-tao closed pull request #3738: [AIRFLOW-2886] Secure Flask SECRET_KEY
URL: https://github.com/apache/incubator-airflow/pull/3738
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/airflow/config_templates/default_airflow.cfg
b/airflow/config_templates/default_airflow.cfg
index b957d41355..7a86e1f069 100644
--- a/airflow/config_templates/default_airflow.cfg
+++ b/airflow/config_templates/default_airflow.cfg
@@ -250,9 +250,8 @@ worker_refresh_batch_size = 1
worker_refresh_interval = 30
# Secret key used to run your flask app
-# If default value is given ("temporary_key"), a random secret_key will be
generated
-# when you launch your webserver for security reason
-secret_key = temporary_key
+# It should be as random as possible
+secret_key = {SECRET_KEY}
# Number of workers to run the Gunicorn web server
workers = 4
diff --git a/airflow/configuration.py b/airflow/configuration.py
index ed8943ac77..9e80648c74 100644
--- a/airflow/configuration.py
+++ b/airflow/configuration.py
@@ -22,6 +22,7 @@
from __future__ import print_function
from __future__ import unicode_literals
+from base64 import b64encode
from builtins import str
from collections import OrderedDict
import copy
@@ -478,6 +479,8 @@ def parameterized_config(template):
else:
FERNET_KEY = ''
+SECRET_KEY = b64encode(os.urandom(16)).decode('utf-8')
+
TEMPLATE_START = (
'# --- TEMPLATE BEGINS HERE ---')
if not os.path.isfile(TEST_CONFIG_FILE):
diff --git a/airflow/www/app.py b/airflow/www/app.py
index 319fe11ada..f7976b0dd5 100644
--- a/airflow/www/app.py
+++ b/airflow/www/app.py
@@ -49,13 +49,7 @@ def create_app(config=None, testing=False):
app = Flask(__name__)
app.wsgi_app = ProxyFix(app.wsgi_app)
-
-if configuration.conf.get('webserver', 'SECRET_KEY') == "temporary_key":
-log.info("SECRET_KEY for Flask App is not specified. Using a random
one.")
-app.secret_key = os.urandom(16)
-else:
-app.secret_key = configuration.conf.get('webserver', 'SECRET_KEY')
-
+app.secret_key = configuration.conf.get('webserver', 'SECRET_KEY')
app.config['LOGIN_DISABLED'] = not configuration.conf.getboolean(
'webserver', 'AUTHENTICATE')
diff --git a/airflow/www_rbac/app.py b/airflow/www_rbac/app.py
index 8d3400a668..b319426aa9 100644
--- a/airflow/www_rbac/app.py
+++ b/airflow/www_rbac/app.py
@@ -43,10 +43,7 @@ def create_app(config=None, session=None, testing=False,
app_name="Airflow"):
global app, appbuilder
app = Flask(__name__)
app.wsgi_app = ProxyFix(app.wsgi_app)
-if conf.get('webserver', 'SECRET_KEY') == "temporary_key":
-app.secret_key = os.urandom(16)
-else:
-app.secret_key = conf.get('webserver', 'SECRET_KEY')
+app.secret_key = conf.get('webserver', 'SECRET_KEY')
airflow_home_path = conf.get('core', 'AIRFLOW_HOME')
webserver_config_path = airflow_home_path + '/webserver_config.py'
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Secure Flask SECRET_KEY
> ---
>
> Key: AIRFLOW-2886
> URL: https://issues.apache.org/jira/browse/AIRFLOW-2886
> Project: Apache Airflow
> Issue Type: Bug
>Reporter: Xiaodong DENG
>Assignee: Xiaodong DENG
>Priority: Critical
>
> In my earlier PRs, [https://github.com/apache/incubator-airflow/pull/3651]
> and [https://github.com/apache/incubator-airflow/pull/3729] , I proposed to
> generate random SECRET_KEY for Flask App.
> If we have multiple workers for the Flask webserver, we may encounter CSRF
> error {{The CSRF session token is missing}} .
> 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).
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (AIRFLOW-2886) Secure Flask SECRET_KEY
[
https://issues.apache.org/jira/browse/AIRFLOW-2886?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16576591#comment-16576591
]
ASF GitHub Bot commented on AIRFLOW-2886:
-
XD-DENG opened a new pull request #3738: [AIRFLOW-2886] Secure Flask SECRET_KEY
URL: https://github.com/apache/incubator-airflow/pull/3738
### Jira
- [x] My PR addresses the following [Airflow
Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references
them in the PR title. For example, "\[AIRFLOW-XXX\] My Airflow PR"
- https://issues.apache.org/jira/browse/AIRFLOW-2886
- In case you are fixing a typo in the documentation you can prepend your
commit with \[AIRFLOW-XXX\], code changes always need a Jira issue.
### Description
- [x] Here are some details about my PR, including screenshots of any UI
changes:
In my earlier PRs, https://github.com/apache/incubator-airflow/pull/3651 and
https://github.com/apache/incubator-airflow/pull/3729 , I proposed to generate
random `SECRET_KEY` for the `webserver` (Flask App).
However, I realise that we may encounter CSRF error `The CSRF session token
is missing` when 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
- [ ] My PR adds the following unit tests __OR__ does not need testing for
this extremely good reason:
### Commits
- [x] My commits all reference Jira issues in their subject lines, and I
have squashed multiple commits if they address the same issue. In addition, my
commits follow the guidelines from "[How to write a good git commit
message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Documentation
- [ ] In case of new functionality, my PR adds documentation that describes
how to use it.
- When adding new operators/hooks/sensors, the autoclass documentation
generation needs to be added.
### Code Quality
- [x] Passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Secure Flask SECRET_KEY
> ---
>
> Key: AIRFLOW-2886
> URL: https://issues.apache.org/jira/browse/AIRFLOW-2886
> Project: Apache Airflow
> Issue Type: Bug
>Reporter: Xiaodong DENG
>Assignee: Xiaodong DENG
>Priority: Critical
>
> In my earlier PRs, [https://github.com/apache/incubator-airflow/pull/3651]
> and [https://github.com/apache/incubator-airflow/pull/3729] , I proposed to
> generate random SECRET_KEY for Flask App.
> If we have multiple workers for the Flask webserver, we may encounter CSRF
> error {{The CSRF session token is missing}} .
> 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).
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
