From b39e236fea33126ddb701a527fe72d339fe4f2e3 Mon Sep 17 00:00:00 2001 From: Shakaib Khan Date: Mon, 27 Sep 2021 14:25:00 -0400 Subject: [PATCH 01/11] Warning of public exposure of deployment in UI with on/off config --- airflow/config_templates/config.yml | 7 +++++++ airflow/config_templates/default_airflow.cfg | 3 +++ airflow/www/views.py | 10 ++++++++++ tests/www/views/test_views_robots.py | 12 ++++++++++++ 4 files changed, 32 insertions(+) diff --git a/airflow/config_templates/config.yml b/airflow/config_templates/config.yml index d0b97308a22e5..c3248a89982c9 100644 --- a/airflow/config_templates/config.yml +++ b/airflow/config_templates/config.yml @@ -1303,6 +1303,13 @@ type: integer example: ~ default: "3" + - name: warn_deployment_exposure + description: | + Boolean for displaying warning for publicly viewable deployment + version_added: 2.2.0 + type: boolean + example: ~ + default: "True" - name: email description: | diff --git a/airflow/config_templates/default_airflow.cfg b/airflow/config_templates/default_airflow.cfg index 496616bbcfb66..f81312b8931e2 100644 --- a/airflow/config_templates/default_airflow.cfg +++ b/airflow/config_templates/default_airflow.cfg @@ -655,6 +655,9 @@ session_lifetime_minutes = 43200 # when auto-refresh is turned on auto_refresh_interval = 3 +# Boolean for displaying warning for publicly viewable deployment +warn_deployment_exposure = True + [email] # Configuration email backend and whether to diff --git a/airflow/www/views.py b/airflow/www/views.py index 8144c9d0481e0..5fa82cd11d87d 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -3091,6 +3091,14 @@ def tree_data(self): # avoid spaces to reduce payload size return htmlsafe_json_dumps(data, separators=(',', ':')) + def warn_deployment_exposure(self): + deployment_warning = "Deployment is exposed to public internet" + flash( + deployment_warning, + "warning", + ) + logging.warning(deployment_warning) + @expose('/robots.txt') @action_logging def robots(self): @@ -3099,6 +3107,8 @@ def robots(self): of the risk associated with exposing Airflow to the public internet, however it does not address the real security risks associated with such a deployment. """ + if conf.getboolean("webserver", "warn_deployment_exposure"): + self.warn_deployment_exposure() return send_from_directory(current_app.static_folder, 'robots.txt') diff --git a/tests/www/views/test_views_robots.py b/tests/www/views/test_views_robots.py index e9fa98666d0e5..a077126754302 100644 --- a/tests/www/views/test_views_robots.py +++ b/tests/www/views/test_views_robots.py @@ -14,8 +14,20 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from tests.test_utils.config import conf_vars def test_robots(viewer_client): resp = viewer_client.get('/robots.txt', follow_redirects=True) assert resp.data.decode('utf-8') == "User-agent: *\nDisallow: /\n" + + +def test_deployment_warning_config(viewer_client): + viewer_client.get('/robots.txt', follow_redirects=True) + resp = viewer_client.get('', follow_redirects=True) + assert "exposed to public internet" in resp.data.decode('utf-8') + + with conf_vars({('webserver', 'warn_deployment_exposure'): 'False'}): + viewer_client.get('/robots.txt', follow_redirects=True) + resp = viewer_client.get('/robots.txt', follow_redirects=True) + assert "exposed to public internet" not in resp.data.decode('utf-8') From 5b4e5568a52c82f7f302e91b49e7021977702514 Mon Sep 17 00:00:00 2001 From: Shakaib Khan Date: Sun, 17 Oct 2021 16:48:46 -0400 Subject: [PATCH 02/11] Now warning admin with more useful warning message --- airflow/www/views.py | 26 ++++++++++++++++---------- tests/www/views/test_views_robots.py | 15 ++++++++------- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/airflow/www/views.py b/airflow/www/views.py index 5fa82cd11d87d..8ea9c649c1b8c 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -788,6 +788,22 @@ def _iter_parsed_moved_data_table_names(): # Second segment is a version marker that we don't need to show. yield segments[2], table_name + warn_deployment_query = session.query(Log).filter(Log.event == "robots").count() + if ( + permissions.ACTION_CAN_ACCESS_MENU, + permissions.RESOURCE_ADMIN_MENU, + ) in user_permissions and warn_deployment_query > 0: + flash( + Markup( + 'Recent requests have been made to /robots.txt. ' + 'This indicates that this deployment may be accessible to the public internet. ' + 'This warning can be disabled by setting webserver.warn_deployment_exposure=False in ' + 'airflow.cfg. Read more about web deployment security here' + ), + "warning", + ) + return self.render_template( 'airflow/dags.html', dags=dags, @@ -3091,14 +3107,6 @@ def tree_data(self): # avoid spaces to reduce payload size return htmlsafe_json_dumps(data, separators=(',', ':')) - def warn_deployment_exposure(self): - deployment_warning = "Deployment is exposed to public internet" - flash( - deployment_warning, - "warning", - ) - logging.warning(deployment_warning) - @expose('/robots.txt') @action_logging def robots(self): @@ -3107,8 +3115,6 @@ def robots(self): of the risk associated with exposing Airflow to the public internet, however it does not address the real security risks associated with such a deployment. """ - if conf.getboolean("webserver", "warn_deployment_exposure"): - self.warn_deployment_exposure() return send_from_directory(current_app.static_folder, 'robots.txt') diff --git a/tests/www/views/test_views_robots.py b/tests/www/views/test_views_robots.py index a077126754302..232bfe03dbfeb 100644 --- a/tests/www/views/test_views_robots.py +++ b/tests/www/views/test_views_robots.py @@ -22,12 +22,13 @@ def test_robots(viewer_client): assert resp.data.decode('utf-8') == "User-agent: *\nDisallow: /\n" -def test_deployment_warning_config(viewer_client): - viewer_client.get('/robots.txt', follow_redirects=True) - resp = viewer_client.get('', follow_redirects=True) - assert "exposed to public internet" in resp.data.decode('utf-8') +def test_deployment_warning_config(admin_client): + warn_text = "webserver.warn_deployment_exposure" + admin_client.get('/robots.txt', follow_redirects=True) + resp = admin_client.get('', follow_redirects=True) + assert warn_text in resp.data.decode('utf-8') with conf_vars({('webserver', 'warn_deployment_exposure'): 'False'}): - viewer_client.get('/robots.txt', follow_redirects=True) - resp = viewer_client.get('/robots.txt', follow_redirects=True) - assert "exposed to public internet" not in resp.data.decode('utf-8') + admin_client.get('/robots.txt', follow_redirects=True) + resp = admin_client.get('/robots.txt', follow_redirects=True) + assert warn_text not in resp.data.decode('utf-8') From 35bdab14ab588332f1880f83f61e61bc9dc1ab82 Mon Sep 17 00:00:00 2001 From: Shakaib Khan Date: Wed, 20 Oct 2021 00:04:31 -0400 Subject: [PATCH 03/11] Increment test view assertion count --- tests/www/views/test_views_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/www/views/test_views_base.py b/tests/www/views/test_views_base.py index cc9968276e006..5759369265fbb 100644 --- a/tests/www/views/test_views_base.py +++ b/tests/www/views/test_views_base.py @@ -30,7 +30,7 @@ def test_index(admin_client): - with assert_queries_count(49): + with assert_queries_count(50): resp = admin_client.get('/', follow_redirects=True) check_content_in_response('DAGs', resp) From 2a8fea96ee1c7ccc526dcc4d89e964c7c72ae1c4 Mon Sep 17 00:00:00 2001 From: Shakaib Khan Date: Wed, 10 Nov 2021 18:18:36 -0500 Subject: [PATCH 04/11] Only check for week old requests and brief description of config --- airflow/www/views.py | 10 +++++++--- docs/apache-airflow/security/webserver.rst | 10 ++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/airflow/www/views.py b/airflow/www/views.py index 8ea9c649c1b8c..72cf599cf5b52 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -121,6 +121,7 @@ from airflow.utils.state import State from airflow.utils.strings import to_boolean from airflow.utils.timezone import td_format +from airflow.utils.timezone import utcnow from airflow.version import version from airflow.www import auth, utils as wwwutils from airflow.www.decorators import action_logging, gzipped @@ -788,18 +789,21 @@ def _iter_parsed_moved_data_table_names(): # Second segment is a version marker that we don't need to show. yield segments[2], table_name - warn_deployment_query = session.query(Log).filter(Log.event == "robots").count() + robots_file_access_count = session.query(Log).filter(Log.event == "robots").filter( + Log.dttm > (utcnow() - timedelta(days=7))).count() + if ( permissions.ACTION_CAN_ACCESS_MENU, permissions.RESOURCE_ADMIN_MENU, - ) in user_permissions and warn_deployment_query > 0: + ) in user_permissions and robots_file_access_count > 0 \ + and conf.getboolean("webserver", "warn_deployment_exposure"): flash( Markup( 'Recent requests have been made to /robots.txt. ' 'This indicates that this deployment may be accessible to the public internet. ' 'This warning can be disabled by setting webserver.warn_deployment_exposure=False in ' 'airflow.cfg. Read more about web deployment security here' + '"https://airflow.apache.org/docs/apache-airflow/stable/security/webserver.html">here ' ), "warning", ) diff --git a/docs/apache-airflow/security/webserver.rst b/docs/apache-airflow/security/webserver.rst index f168faaedc47e..72bc6bc54d203 100644 --- a/docs/apache-airflow/security/webserver.rst +++ b/docs/apache-airflow/security/webserver.rst @@ -31,6 +31,16 @@ set the below: [webserver] x_frame_enabled = False +Disable Deployment Exposure Warning +------------------------------------------------------ + +Airflow warns when recent requests are made to /robot.txt. To disable this warning set the bewlow: + +.. code-block:: ini + + [webserver] + warn_deployment_exposure = False + Sensitive Variable fields ------------------------- From 8d31d675828220bd14b4e1e89c0d38e412192eaa Mon Sep 17 00:00:00 2001 From: Shakaib Khan Date: Mon, 15 Nov 2021 18:45:06 -0500 Subject: [PATCH 05/11] spelling mistake --- docs/apache-airflow/security/webserver.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/apache-airflow/security/webserver.rst b/docs/apache-airflow/security/webserver.rst index 72bc6bc54d203..6cc5de2c59a5f 100644 --- a/docs/apache-airflow/security/webserver.rst +++ b/docs/apache-airflow/security/webserver.rst @@ -34,7 +34,7 @@ set the below: Disable Deployment Exposure Warning ------------------------------------------------------ -Airflow warns when recent requests are made to /robot.txt. To disable this warning set the bewlow: +Airflow warns when recent requests are made to /robot.txt. To disable this warning set the below: .. code-block:: ini From 709a5832d446b657fb0ee5be4424075cf3519c57 Mon Sep 17 00:00:00 2001 From: Shakaib Khan Date: Mon, 15 Nov 2021 20:11:45 -0500 Subject: [PATCH 06/11] only query if warn_deployment is active and shorten line --- airflow/www/views.py | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/airflow/www/views.py b/airflow/www/views.py index 72cf599cf5b52..f67e2fb885711 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -789,24 +789,28 @@ def _iter_parsed_moved_data_table_names(): # Second segment is a version marker that we don't need to show. yield segments[2], table_name - robots_file_access_count = session.query(Log).filter(Log.event == "robots").filter( - Log.dttm > (utcnow() - timedelta(days=7))).count() - if ( permissions.ACTION_CAN_ACCESS_MENU, permissions.RESOURCE_ADMIN_MENU, - ) in user_permissions and robots_file_access_count > 0 \ - and conf.getboolean("webserver", "warn_deployment_exposure"): - flash( - Markup( - 'Recent requests have been made to /robots.txt. ' - 'This indicates that this deployment may be accessible to the public internet. ' - 'This warning can be disabled by setting webserver.warn_deployment_exposure=False in ' - 'airflow.cfg. Read more about web deployment security here ' - ), - "warning", + ) in user_permissions and conf.getboolean("webserver", "warn_deployment_exposure"): + robots_file_access_count = ( + session.query(Log) + .filter(Log.event == "robots") + .filter(Log.dttm > (utcnow() - timedelta(days=7))) + .count() ) + if robots_file_access_count > 0: + flash( + Markup( + 'Recent requests have been made to /robots.txt. ' + 'This indicates that this deployment may be accessible to the public internet. ' + 'This warning can be disabled by setting webserver.warn_deployment_exposure=False in ' + 'airflow.cfg. Read more about web deployment security ' + 'here' + ), + "warning", + ) return self.render_template( 'airflow/dags.html', From d816f2f66311d66bf806c23b2684cfc3c3d321d2 Mon Sep 17 00:00:00 2001 From: Shakaib Khan Date: Sat, 11 Dec 2021 15:44:32 -0500 Subject: [PATCH 07/11] use get_doc_url and format docs --- airflow/config_templates/config.yml | 2 +- airflow/www/views.py | 2 +- docs/apache-airflow/security/webserver.rst | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/airflow/config_templates/config.yml b/airflow/config_templates/config.yml index c3248a89982c9..f929e738dbc5d 100644 --- a/airflow/config_templates/config.yml +++ b/airflow/config_templates/config.yml @@ -1306,7 +1306,7 @@ - name: warn_deployment_exposure description: | Boolean for displaying warning for publicly viewable deployment - version_added: 2.2.0 + version_added: 2.3.0 type: boolean example: ~ default: "True" diff --git a/airflow/www/views.py b/airflow/www/views.py index f67e2fb885711..3d1aeb9735cb2 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -806,7 +806,7 @@ def _iter_parsed_moved_data_table_names(): 'This indicates that this deployment may be accessible to the public internet. ' 'This warning can be disabled by setting webserver.warn_deployment_exposure=False in ' 'airflow.cfg. Read more about web deployment security ' + f'"{get_docs_url("security/webserver.html")}">' 'here' ), "warning", diff --git a/docs/apache-airflow/security/webserver.rst b/docs/apache-airflow/security/webserver.rst index 6cc5de2c59a5f..9b7ba8c81afe3 100644 --- a/docs/apache-airflow/security/webserver.rst +++ b/docs/apache-airflow/security/webserver.rst @@ -32,9 +32,10 @@ set the below: x_frame_enabled = False Disable Deployment Exposure Warning ------------------------------------------------------- +--------------------------------------- -Airflow warns when recent requests are made to /robot.txt. To disable this warning set the below: +Airflow warns when recent requests are made to `/robot.txt`. To disable this warning set ``warn_deployment_exposure`` to +``False`` as below: .. code-block:: ini From 1e4df8ef8dc0092d2b4e150d1f16fb90a40da8f8 Mon Sep 17 00:00:00 2001 From: Shakaib Khan Date: Sat, 11 Dec 2021 16:05:42 -0500 Subject: [PATCH 08/11] remove double ticks from webserver rst --- docs/apache-airflow/security/webserver.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/apache-airflow/security/webserver.rst b/docs/apache-airflow/security/webserver.rst index 9b7ba8c81afe3..dfff52e3403a3 100644 --- a/docs/apache-airflow/security/webserver.rst +++ b/docs/apache-airflow/security/webserver.rst @@ -34,8 +34,8 @@ set the below: Disable Deployment Exposure Warning --------------------------------------- -Airflow warns when recent requests are made to `/robot.txt`. To disable this warning set ``warn_deployment_exposure`` to -``False`` as below: +Airflow warns when recent requests are made to `/robot.txt`. To disable this warning set `warn_deployment_exposure` to +`False` as below: .. code-block:: ini From ddc876bde5fd8991c3d084777c38a694dd4da08a Mon Sep 17 00:00:00 2001 From: Shakaib Khan Date: Sat, 11 Dec 2021 19:39:50 -0500 Subject: [PATCH 09/11] Add double quote to webserver.rst --- docs/apache-airflow/security/webserver.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/apache-airflow/security/webserver.rst b/docs/apache-airflow/security/webserver.rst index dfff52e3403a3..9b7ba8c81afe3 100644 --- a/docs/apache-airflow/security/webserver.rst +++ b/docs/apache-airflow/security/webserver.rst @@ -34,8 +34,8 @@ set the below: Disable Deployment Exposure Warning --------------------------------------- -Airflow warns when recent requests are made to `/robot.txt`. To disable this warning set `warn_deployment_exposure` to -`False` as below: +Airflow warns when recent requests are made to `/robot.txt`. To disable this warning set ``warn_deployment_exposure`` to +``False`` as below: .. code-block:: ini From a3225b15b51055c9d5200459684e867a6ff24dda Mon Sep 17 00:00:00 2001 From: Kaxil Naik Date: Thu, 16 Dec 2021 01:22:25 +0000 Subject: [PATCH 10/11] Apply suggestions from code review --- docs/apache-airflow/security/webserver.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/apache-airflow/security/webserver.rst b/docs/apache-airflow/security/webserver.rst index 9b7ba8c81afe3..91a06ad1235ea 100644 --- a/docs/apache-airflow/security/webserver.rst +++ b/docs/apache-airflow/security/webserver.rst @@ -34,7 +34,7 @@ set the below: Disable Deployment Exposure Warning --------------------------------------- -Airflow warns when recent requests are made to `/robot.txt`. To disable this warning set ``warn_deployment_exposure`` to +Airflow warns when recent requests are made to ``/robot.txt``. To disable this warning set ``warn_deployment_exposure`` to ``False`` as below: .. code-block:: ini From a21b952942f65a4b68691c4719926a6dcdcfbb66 Mon Sep 17 00:00:00 2001 From: Shakaib Khan Date: Sun, 19 Dec 2021 11:01:05 -0500 Subject: [PATCH 11/11] Update airflow/www/views.py Co-authored-by: eladkal <45845474+eladkal@users.noreply.github.com> --- airflow/www/views.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/airflow/www/views.py b/airflow/www/views.py index 3d1aeb9735cb2..388e69cd0f70f 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -120,8 +120,7 @@ from airflow.utils.session import create_session, provide_session from airflow.utils.state import State from airflow.utils.strings import to_boolean -from airflow.utils.timezone import td_format -from airflow.utils.timezone import utcnow +from airflow.utils.timezone import td_format, utcnow from airflow.version import version from airflow.www import auth, utils as wwwutils from airflow.www.decorators import action_logging, gzipped