Decrypt SecureString value obtained by SsmHook

[Taragolis]

Right now if we try to get SecureString by SsmHook.get_parameter_value then we get encrypted value, which is useless for further usage without decryption.

from airflow.providers.amazon.aws.hooks.ssm import SsmHook

hook = SsmHook(aws_conn_id=None, region_name="eu-west-1")
print(hook.get_parameter_value("/airflow/config/boom"))

Without this changes

AQICAHiA4cbp5//uho/.../Mlb3cleB4/7XXjkh

After this changes

postgresql+psycopg2://airflow:insecurepassword@postgres/airflow

WDYT, should we also mask value if value has type SecureString?

[ferruzzi]

Nice catch! IMHO, if we are decoding by default then masking sounds like the right answer to me. I'm not really up to date on best practices when using SecureString though, so I'm happy to defer if someone feels otherwise.

[Taragolis]

Nice catch! IMHO, if we are decoding by default then masking sounds like the right answer to me. I'm not really up to date on best practices when using SecureString though, so I'm happy to defer if someone feels otherwise.

Well there is not easy answer as well as best practices. We do not know what users might store into SSM Parameter Store and how they intend to use it.

If it credentials the answer straightforward, yes we should, like here:

airflow/airflow/providers/amazon/aws/hooks/base_aws.py

Lines 662 to 666 in 3b25168

	creds = self.get_session(region_name=region_name).get_credentials().get_frozen_credentials()
	mask_secret(creds.secret_key)
	if creds.token:
	mask_secret(creds.token)
	return creds

IMHO, In general if you create secure string you do not want to some one who does not have access to KMS keys see value.
But we could mask all or nothing, that mean postgresql+psycopg2://airflow:insecurepassword@postgres/airflow in logs transform to ***

[ferruzzi]

Nice catch! IMHO, if we are decoding by default then masking sounds like the right answer to me. I'm not really up to date on best practices when using SecureString though, so I'm happy to defer if someone feels otherwise.

Well there is not easy answer as well as best practices. We do not know what users might store into SSM Parameter Store and how they intend to use it.

If it credentials the answer straightforward, yes we should, like here:

airflow/airflow/providers/amazon/aws/hooks/base_aws.py

Lines 662 to 666 in 3b25168

	creds = self.get_session(region_name=region_name).get_credentials().get_frozen_credentials()
	mask_secret(creds.secret_key)
	if creds.token:
	mask_secret(creds.token)
	return creds

IMHO, In general if you create secure string you do not want to some one who does not have access to KMS keys see value. But we could mask all or nothing, that mean postgresql+psycopg2://airflow:insecurepassword@postgres/airflow in logs transform to ***

It would be ideal if only the password got masked, but I think if a user is setting the parameter as a secure string, it would be better to assume more security than less.

[Taragolis]

Since we cant be sure where it password and where it is not than better mask all.
I will ad it tomorrow.

[ferruzzi]

Nice, thanks for adding the masking.

[Taragolis]

@o-nikolas if you have a time could you also have a look (you have a binding approval power) and maybe we could merge it

[eladkal]

LGTM