Add config to warn public deployment exposure in UI #18557

ShakaibKhan · 2021-09-27T21:21:28Z

Added warning banner message for when /robot.txt is hit with on/off config

potiuk · 2021-09-28T16:06:49Z

There were some delays in night which caused waitign for image to timeout. Close/reopen to rebuild

SamWheating · 2021-09-28T16:52:52Z

airflow/www/views.py

Could this message maybe be extended with some more specific or useful information?

Maybe something like:

Recent requests to /robots.txt indicate that this deployment may be accessible to the public internet. This warning can be disabled by setting webserver.warn_deployment_exposure=False in the configuration.

We could also link to a page in the docs with more information on webserver security.

mik-laj · 2021-09-28T18:04:04Z

airflow/www/views.py

Flask uses cookie-based sessions. This warning will therefore only be displayed to clients who have visited /robots.txt, in our case bot crawlers. I think we should notify the administrator about this situation, not the bot.

what would be the best way to display the message for admin team? is there some way to post the message only for admin sessions?

I guess you need to write it down to the database as a new table or to the event log table.

ShakaibKhan · 2021-11-01T21:42:05Z

quarantined test are failing but having trouble running them locally to fix

mik-laj · 2021-11-02T09:11:27Z

@ShakaibKhan please ignore quarantined tests.

potiuk · 2021-11-08T18:52:44Z

LGTM. @mik-laj @SamWheating ?

SamWheating · 2021-11-08T20:48:27Z

airflow/www/views.py

Should this be limited to the last week or something? The warning message explicitly states that Recent requests have been made to /robots.txt but this could be triggered by very old requests.

There's also no way to clear this entry without deleting the log event so it might persist for a long time, even if the vulnerability is fixed.

Wondering if it would be good enough to clear requests older than a week and only display the message on recent requests

Could we also just query for events within one week old? Something like:

robots_file_access_count = ( session.query(Log) .filter(Log.event == "robots") .filter(Log.dttm > (utcnow() - timedelta(days=7))) .count() )

SamWheating · 2021-11-08T20:51:07Z

airflow/www/views.py

I don't think this warning can be disabled? I don't see the value of warn_deployment_exposure actually being referenced anywhere other than the config.

removed it during a merge, I will put it back in the if statement

mik-laj · 2021-11-08T21:02:30Z

airflow/www/views.py

In one sentence, we are talking about a configuration option, and then we link to a documentation that does not contain a description of that option.

I will add the config to documentation

SamWheating · 2021-11-16T00:47:41Z

airflow/www/views.py

This query should only be run if webserver.warn_deployment_exposure is enabled. Otherwise its putting additional load on the database for no reason, since the warning isn't going to be displayed anyways.

github-actions · 2021-12-08T23:46:44Z

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

kaxil · 2021-12-08T23:46:46Z

Can you review this again @SamWheating to see if your comments are addressed

mik-laj · 2021-12-09T00:23:26Z

airflow/www/views.py

Can you use the get_docs_url function? Thanks to it, the link will be pinned to the version.

Probably:

Suggested change

'"https://airflow.apache.org/docs/apache-airflow/stable/security/webserver.html">'

f'"{get_docs_url('security/webserver.html')}">'

SamWheating · 2021-12-09T00:41:05Z

airflow/config_templates/config.yml

Suggested change

version_added: 2.2.0

version_added: 2.2.3

I think that this value will need to be updated, but if this is merged in the next week or two it will probably be included in the 2.2.3 release.

Let's make it 2.3.0 please -- this won't be included in 2.2.3

Suggested change

version_added: 2.2.0

version_added: 2.3.0

kaxil · 2021-12-09T13:30:12Z

docs/apache-airflow/security/webserver.rst

Suggested change

Disable Deployment Exposure Warning

------------------------------------------------------

Disable Deployment Exposure Warning

---------------------------------------

kaxil · 2021-12-09T13:30:23Z

docs/apache-airflow/security/webserver.rst

Suggested change

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:

docs/apache-airflow/security/webserver.rst

potiuk · 2021-12-18T17:37:18Z

Needs conflict resolving :(

airflow/www/views.py

Co-authored-by: eladkal <[email protected]>

ShakaibKhan requested review from ashb, bbovenzi and ryanahamilton as code owners September 27, 2021 21:21

boring-cyborg bot added the area:webserver Webserver related Issues label Sep 27, 2021

ShakaibKhan mentioned this pull request Sep 27, 2021

Warn if robots.txt is accessed #17962

Closed

2 tasks

potiuk closed this Sep 28, 2021

potiuk reopened this Sep 28, 2021

SamWheating reviewed Sep 28, 2021

View reviewed changes

mik-laj reviewed Sep 28, 2021

View reviewed changes

ShakaibKhan force-pushed the warn_public_deployment branch 2 times, most recently from 3e41c64 to 001298d Compare October 20, 2021 04:48

ShakaibKhan force-pushed the warn_public_deployment branch from 001298d to f584371 Compare October 25, 2021 18:21

SamWheating reviewed Nov 8, 2021

View reviewed changes

mik-laj reviewed Nov 8, 2021

View reviewed changes

ShakaibKhan force-pushed the warn_public_deployment branch from f584371 to aafffd5 Compare November 10, 2021 23:21

ShakaibKhan requested a review from kaxil as a code owner November 10, 2021 23:21

SamWheating reviewed Nov 16, 2021

View reviewed changes

kaxil requested a review from mik-laj December 8, 2021 23:44

kaxil approved these changes Dec 8, 2021

View reviewed changes

kaxil requested a review from uranusjr December 8, 2021 23:46

github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Dec 8, 2021

mik-laj reviewed Dec 9, 2021

View reviewed changes

SamWheating reviewed Dec 9, 2021

View reviewed changes

kaxil reviewed Dec 9, 2021

View reviewed changes

ShakaibKhan force-pushed the warn_public_deployment branch from d70a162 to 8dfca34 Compare December 11, 2021 21:05

ShakaibKhan requested a review from potiuk as a code owner December 11, 2021 21:05

kaxil reviewed Dec 16, 2021

View reviewed changes

docs/apache-airflow/security/webserver.rst Outdated Show resolved Hide resolved

kaxil requested review from dstandish, eladkal and mik-laj December 16, 2021 01:22

Shakaib Khan and others added 10 commits December 18, 2021 12:46

Warning of public exposure of deployment in UI with on/off config

b39e236

Now warning admin with more useful warning message

5b4e556

Increment test view assertion count

35bdab1

Only check for week old requests and brief description of config

2a8fea9

spelling mistake

8d31d67

only query if warn_deployment is active and shorten line

709a583

use get_doc_url and format docs

d816f2f

remove double ticks from webserver rst

1e4df8e

Add double quote to webserver.rst

ddc876b

Apply suggestions from code review

a3225b1

ShakaibKhan force-pushed the warn_public_deployment branch from ffc8ade to a3225b1 Compare December 18, 2021 17:53

eladkal reviewed Dec 19, 2021

View reviewed changes

airflow/www/views.py Outdated Show resolved Hide resolved

Update airflow/www/views.py

a21b952

Co-authored-by: eladkal <[email protected]>

potiuk approved these changes Dec 19, 2021

View reviewed changes

uranusjr changed the title ~~Warning of public exposure of deployment in UI with on/off config~~ Add config to warn public deployment exposure in UI Dec 24, 2021

uranusjr merged commit 24a53fb into apache:main Dec 24, 2021

ephraimbuddy added the type:new-feature Changelog: New Features label Apr 14, 2022

gmsantos mentioned this pull request May 10, 2022

Add index for event column in log table #23625

Merged

	'"https://airflow.apache.org/docs/apache-airflow/stable/security/webserver.html">'
	f'"{get_docs_url('security/webserver.html')}">'

-Disable Deployment Exposure Warning
-------------------------------------------------------
+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:

Add config to warn public deployment exposure in UI #18557

Add config to warn public deployment exposure in UI #18557

Uh oh!

Conversation

ShakaibKhan commented Sep 27, 2021 • edited by eladkal Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

potiuk commented Sep 28, 2021

Uh oh!

SamWheating Sep 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShakaibKhan commented Nov 1, 2021

Uh oh!

mik-laj commented Nov 2, 2021

Uh oh!

potiuk commented Nov 8, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SamWheating Nov 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SamWheating Nov 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShakaibKhan Nov 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 8, 2021

Uh oh!

kaxil commented Dec 8, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaxil Dec 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

potiuk commented Dec 18, 2021

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ShakaibKhan commented Sep 27, 2021 •

edited by eladkal

Loading

SamWheating Sep 28, 2021 •

edited

Loading

SamWheating Nov 9, 2021 •

edited

Loading

SamWheating Nov 8, 2021 •

edited

Loading

ShakaibKhan Nov 9, 2021 •

edited

Loading

kaxil Dec 9, 2021 •

edited

Loading