You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/11/09 21:17:44 UTC
[GitHub] [airflow] josh-fell opened a new pull request #19497: Add "access_key" to DEFAULT_SENSITIVE_FIELDS
josh-fell opened a new pull request #19497:
URL: https://github.com/apache/airflow/pull/19497
Users who call the `WasbHook` and authenticate with a Shared Access Key to Azure Blob Storage will see their Shared Access Key in plain text within the Task Instance logs. Unfortunately since `Host` is not masked and is required to authenticate with a Shared Access Key, the entirety of the supplied credentials are written in the log.
**Sample log entry before**
```shell
[2021-11-09, 20:49:52 UTC] {base.py:79} INFO - Using connection to: id: wasb_default. Host: https://myAccountBlahBlah.blob.core.windows.net/, Port: None, Schema: , Login: , Password: None, extra: {'extra__wasb__connection_string': '', 'extra__wasb__sas_token': '***', 'extra__wasb__shared_access_key': 'KEa1QvMjxMNLuzTgVZFvS6PpQv087Ls0Oq+7Ic/fa9Lu3RQwunHi61yZTCJSCIo1gZBNLuzTgVZFvS6PpQv087Ls0Oq+7Ic/fa9Lu3RQwunHi61yZTCJSCIo1gZBH1cc/KZb3EAKXrqWXXXXXXXXXXXXXXXXXXXXXX==', 'extra__wasb__tenant_id': ''}
```
**Sample log entry after**
```shell
[2021-11-09, 20:56:37 UTC] {base.py:79} INFO - Using connection to: id: wasb_default. Host: https://myAccountBlahBlah.blob.core.windows.net/, Port: None, Schema: , Login: , Password: None, extra: {'extra__wasb__connection_string': '', 'extra__wasb__sas_token': '***', 'extra__wasb__shared_access_key': '***', 'extra__wasb__tenant_id': ''}
```
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #19497: Add "access_key" to DEFAULT_SENSITIVE_FIELDS
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #19497:
URL: https://github.com/apache/airflow/pull/19497#discussion_r746504218
##########
File path: airflow/utils/log/secrets_masker.py
##########
@@ -34,6 +34,7 @@
DEFAULT_SENSITIVE_FIELDS = frozenset(
{
'access_token',
+ 'access_key',
Review comment:
I'm slightly sceptical if we should even have "access_key" in this list -- at least for AWS connections (the most common source of this) the access_key is not private.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] josh-fell commented on a change in pull request #19497: Add "access_key" to DEFAULT_SENSITIVE_FIELDS
Posted by GitBox <gi...@apache.org>.
josh-fell commented on a change in pull request #19497:
URL: https://github.com/apache/airflow/pull/19497#discussion_r746730866
##########
File path: airflow/utils/log/secrets_masker.py
##########
@@ -34,6 +34,7 @@
DEFAULT_SENSITIVE_FIELDS = frozenset(
{
'access_token',
+ 'access_key',
Review comment:
Two other thoughts though:
- Is "shared_access_key" a better entry in the list?
- Does the `{base.py:79} INFO - Using connection to: id: wasb_default. Host: https:...` line need to be so verbose? Is shortening this to `INFO - Using connection to: id: wasb_default. ` sufficient?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] josh-fell commented on a change in pull request #19497: Add "access_key" to DEFAULT_SENSITIVE_FIELDS
Posted by GitBox <gi...@apache.org>.
josh-fell commented on a change in pull request #19497:
URL: https://github.com/apache/airflow/pull/19497#discussion_r746055063
##########
File path: airflow/utils/log/secrets_masker.py
##########
@@ -34,6 +34,7 @@
DEFAULT_SENSITIVE_FIELDS = frozenset(
{
'access_token',
+ 'access_key',
Review comment:
Thinking if "key" should be added? This would be the 4th entry with "key" in the name and generic "secret" and "token" are also in this set. But perhaps it is _too_ general of a term.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] josh-fell commented on a change in pull request #19497: Add "access_key" to DEFAULT_SENSITIVE_FIELDS
Posted by GitBox <gi...@apache.org>.
josh-fell commented on a change in pull request #19497:
URL: https://github.com/apache/airflow/pull/19497#discussion_r755697566
##########
File path: airflow/utils/log/secrets_masker.py
##########
@@ -34,6 +34,7 @@
DEFAULT_SENSITIVE_FIELDS = frozenset(
{
'access_token',
+ 'access_key',
Review comment:
I'll close this PR and open an issue to bring in some discussion about what a proper fix could/should be. (Unless you think updates related to any of the above 2 questions would suffice.)
>I wonder if this is a "bug" in WASM connection, and it should be using password for the secret key here?
Obviously some tradeoff between reusing a connection field vs making the connection form more straightforward. Has there typically been deference in a certain direction historically?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] josh-fell commented on a change in pull request #19497: Add "access_key" to DEFAULT_SENSITIVE_FIELDS
Posted by GitBox <gi...@apache.org>.
josh-fell commented on a change in pull request #19497:
URL: https://github.com/apache/airflow/pull/19497#discussion_r746728905
##########
File path: airflow/utils/log/secrets_masker.py
##########
@@ -34,6 +34,7 @@
DEFAULT_SENSITIVE_FIELDS = frozenset(
{
'access_token',
+ 'access_key',
Review comment:
Yep, fair point. Although when the AWS Access Key is written to the logs I imagine it's not masked since its specified as `login` in the connection. Only if the access key is added to `Extras` is the value masked (although the [connection doc](https://airflow.apache.org/docs/apache-airflow-providers-amazon/stable/connections/aws.html) for AWS doesn't specify using `Extras` in this manner) -- seems unlikely this approach is used.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] josh-fell commented on a change in pull request #19497: Add "access_key" to DEFAULT_SENSITIVE_FIELDS
Posted by GitBox <gi...@apache.org>.
josh-fell commented on a change in pull request #19497:
URL: https://github.com/apache/airflow/pull/19497#discussion_r746055063
##########
File path: airflow/utils/log/secrets_masker.py
##########
@@ -34,6 +34,7 @@
DEFAULT_SENSITIVE_FIELDS = frozenset(
{
'access_token',
+ 'access_key',
Review comment:
Generally thinking if "key" should be added? This would be the 4th entry with "key" in the name and generic "secret" and "token" are also in this set. But perhaps it is _too_ general of a term.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #19497: Add "access_key" to DEFAULT_SENSITIVE_FIELDS
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #19497:
URL: https://github.com/apache/airflow/pull/19497#discussion_r746730540
##########
File path: airflow/utils/log/secrets_masker.py
##########
@@ -34,6 +34,7 @@
DEFAULT_SENSITIVE_FIELDS = frozenset(
{
'access_token',
+ 'access_key',
Review comment:
I wonder if this is a "bug" in WASM connection, and it should be using password for the secret key here?
(A long time ago AWS _used_ to use extra instead of login/password)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] josh-fell closed pull request #19497: Add "access_key" to DEFAULT_SENSITIVE_FIELDS
Posted by GitBox <gi...@apache.org>.
josh-fell closed pull request #19497:
URL: https://github.com/apache/airflow/pull/19497
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org