diff --git a/CHANGELOG.md b/CHANGELOG.md index a17ec282622..488434095b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ The types of changes are: * Home screen header scaling and responsiveness issues [#2200](https://github.com/ethyca/fides/pull/2277) * Added a feature flag for the recent dataset classification UX changes [#2335](https://github.com/ethyca/fides/pull/2335) * Privacy Center identity inputs validate even when they are optional. [#2308](https://github.com/ethyca/fides/pull/2308) +* Patch masking strategies to better handle null and non-string inputs [#2307](https://github.com/ethyca/fides/pull/2377) ### Security diff --git a/src/fides/api/ops/service/masking/strategy/masking_strategy_aes_encrypt.py b/src/fides/api/ops/service/masking/strategy/masking_strategy_aes_encrypt.py index 36d539bc304..bdf7dcb2cfb 100644 --- a/src/fides/api/ops/service/masking/strategy/masking_strategy_aes_encrypt.py +++ b/src/fides/api/ops/service/masking/strategy/masking_strategy_aes_encrypt.py @@ -37,9 +37,10 @@ def __init__(self, configuration: AesEncryptionMaskingConfiguration): def mask( self, values: Optional[List[str]], request_id: Optional[str] - ) -> Optional[List[str]]: + ) -> Optional[List[Optional[str]]]: if values is None: return None + if self.mode == AesEncryptionMaskingConfiguration.Mode.GCM: masking_meta: Dict[ SecretType, MaskingSecretMeta @@ -57,12 +58,16 @@ def mask( # and therefore the same masked val through the aes strategy. This is called convergent encryption, with this # implementation loosely based on https://www.vaultproject.io/docs/secrets/transit#convergent-encryption - masked_values: List[str] = [] + masked_values: List[Optional[str]] = [] for value in values: + if value is None: + masked_values.append(None) + continue + nonce: bytes | None = self._generate_nonce( - value, key_hmac, request_id, masking_meta # type: ignore + str(value), key_hmac, request_id, masking_meta # type: ignore ) - masked: str = encrypt(value, key, nonce) # type: ignore + masked: str = encrypt(str(value), key, nonce) # type: ignore if self.format_preservation is not None: formatter = FormatPreservation(self.format_preservation) masked = formatter.format(masked) diff --git a/src/fides/api/ops/service/masking/strategy/masking_strategy_hash.py b/src/fides/api/ops/service/masking/strategy/masking_strategy_hash.py index e32ab414e15..aef14b1a1eb 100644 --- a/src/fides/api/ops/service/masking/strategy/masking_strategy_hash.py +++ b/src/fides/api/ops/service/masking/strategy/masking_strategy_hash.py @@ -42,11 +42,12 @@ def __init__( def mask( self, values: Optional[List[str]], request_id: Optional[str] - ) -> Optional[List[str]]: + ) -> Optional[List[Optional[str]]]: """Returns the hashed version of the provided values. Returns None if the provided value is None""" if values is None: return None + masking_meta: Dict[ SecretType, MaskingSecretMeta ] = self._build_masking_secret_meta() @@ -56,9 +57,13 @@ def mask( masking_meta[SecretType.salt], ) - masked_values: List[str] = [] + masked_values: List[Optional[str]] = [] for value in values: - masked: str = self.algorithm_function(value, salt) # type: ignore + if value is None: + masked_values.append(None) + continue + + masked: str = self.algorithm_function(str(value), salt) # type: ignore if self.format_preservation is not None: formatter = FormatPreservation(self.format_preservation) masked = formatter.format(masked) diff --git a/src/fides/api/ops/service/masking/strategy/masking_strategy_hmac.py b/src/fides/api/ops/service/masking/strategy/masking_strategy_hmac.py index f76713b70d8..7b542d8c494 100644 --- a/src/fides/api/ops/service/masking/strategy/masking_strategy_hmac.py +++ b/src/fides/api/ops/service/masking/strategy/masking_strategy_hmac.py @@ -37,13 +37,14 @@ def __init__( def mask( self, values: Optional[List[str]], request_id: Optional[str] - ) -> Optional[List[str]]: + ) -> Optional[List[Optional[str]]]: """ Returns a hash using the hmac algorithm, generating a hash of each of the supplied value and the secret hmac_key. Returns None if the provided value is None. """ if values is None: return None + masking_meta: Dict[ SecretType, MaskingSecretMeta ] = self._build_masking_secret_meta() @@ -54,9 +55,12 @@ def mask( request_id, SecretType.salt, masking_meta[SecretType.salt] ) - masked_values: List[str] = [] + masked_values: List[Optional[str]] = [] for value in values: - masked: str = hmac_encrypt_return_str(value, key, salt, self.algorithm) # type: ignore + if value is None: + masked_values.append(None) + continue + masked: str = hmac_encrypt_return_str(str(value), key, salt, self.algorithm) # type: ignore if self.format_preservation is not None: formatter = FormatPreservation(self.format_preservation) masked = formatter.format(masked) diff --git a/src/fides/api/ops/service/masking/strategy/masking_strategy_random_string_rewrite.py b/src/fides/api/ops/service/masking/strategy/masking_strategy_random_string_rewrite.py index 9a4e0ddd62d..a0e5e6c6ec1 100644 --- a/src/fides/api/ops/service/masking/strategy/masking_strategy_random_string_rewrite.py +++ b/src/fides/api/ops/service/masking/strategy/masking_strategy_random_string_rewrite.py @@ -34,6 +34,7 @@ def mask( """Replaces the value with a random lowercase string of the configured length""" if values is None: return None + masked_values: List[str] = [] for _ in range(len(values)): masked: str = "".join( diff --git a/tests/ops/service/masking/strategy/test_masking_strategy_aes_encrypt.py b/tests/ops/service/masking/strategy/test_masking_strategy_aes_encrypt.py index c5ab8e90fe1..58b85356e84 100644 --- a/tests/ops/service/masking/strategy/test_masking_strategy_aes_encrypt.py +++ b/tests/ops/service/masking/strategy/test_masking_strategy_aes_encrypt.py @@ -1,3 +1,4 @@ +from datetime import datetime from unittest import mock from unittest.mock import Mock @@ -27,9 +28,10 @@ def test_mask_gcm_happypath(mock_encrypt: Mock): cache_secrets() masked_value = AES_STRATEGY.mask(["value"], request_id)[0] - mock_encrypt.assert_called_with( - "value", b"\x94Y\xa8Z", b"\x94Y\xa8Z\xd9\x12\x83\x00\xa4~\ny" + "value", + b"y\xc5I\xd4\x92\xf6G\t\x80\xb1$\x06\x19t/\xc4", + b"\x94Y\xa8Z\xd9\x12\x83\x00\xa4~\ny", ) assert masked_value == mock_encrypt.return_value clear_cache_secrets(request_id) @@ -49,7 +51,7 @@ def test_mask_all_aes_modes(mock_encrypt: Mock): def cache_secrets() -> None: secret_key = MaskingSecretCache[bytes]( - secret=b"\x94Y\xa8Z", + secret=b"y\xc5I\xd4\x92\xf6G\t\x80\xb1$\x06\x19t/\xc4", masking_strategy=AesEncryptionMaskingStrategy.name, secret_type=SecretType.key, ) @@ -66,3 +68,27 @@ def cache_secrets() -> None: secret_type=SecretType.salt_hmac, ) cache_secret(secret_hmac_salt, request_id) + + +def test_mask_arguments_null_list(): + configuration = AesEncryptionMaskingConfiguration() + masker = AesEncryptionMaskingStrategy(configuration) + expected = [None] + + cache_secrets() + + masked = masker.mask([None], request_id) + assert expected == masked + clear_cache_secrets(request_id) + + +def test_mask_arguments_date(): + configuration = AesEncryptionMaskingConfiguration() + masker = AesEncryptionMaskingStrategy(configuration) + expected = ["Sb9RzoQls/Nymd23qY4ZXoy/HBDrZAxeRgKNYv5LwwxlsPE="] + + cache_secrets() + + masked = masker.mask([datetime(2000, 1, 1)], request_id) + assert expected == masked + clear_cache_secrets(request_id) diff --git a/tests/ops/service/masking/strategy/test_masking_strategy_hash.py b/tests/ops/service/masking/strategy/test_masking_strategy_hash.py index 660be1fe508..de467b0b67a 100644 --- a/tests/ops/service/masking/strategy/test_masking_strategy_hash.py +++ b/tests/ops/service/masking/strategy/test_masking_strategy_hash.py @@ -1,8 +1,7 @@ +from datetime import datetime + from fides.api.ops.schemas.masking.masking_configuration import HashMaskingConfiguration from fides.api.ops.schemas.masking.masking_secrets import MaskingSecretCache, SecretType -from fides.api.ops.service.masking.strategy.masking_strategy_aes_encrypt import ( - AesEncryptionMaskingStrategy, -) from fides.api.ops.service.masking.strategy.masking_strategy_hash import ( HashMaskingStrategy, ) @@ -97,3 +96,37 @@ def test_mask_arguments_null(): masked = masker.mask(None, request_id) assert expected == masked clear_cache_secrets(request_id) + + +def test_mask_arguments_null_in_list(): + configuration = HashMaskingConfiguration() + masker = HashMaskingStrategy(configuration) + expected = [None] + + secret = MaskingSecretCache[str]( + secret="adobo", + masking_strategy=HashMaskingStrategy.name, + secret_type=SecretType.salt, + ) + cache_secret(secret, request_id) + + masked = masker.mask([None], request_id) + assert expected == masked + clear_cache_secrets(request_id) + + +def test_mask_datetime(): + configuration = HashMaskingConfiguration() + masker = HashMaskingStrategy(configuration) + expected = ["a6597d576d8fb7ff58047db31f6c526bf984db454fa2460cdf7cf4f9d72a6d09"] + + secret = MaskingSecretCache[str]( + secret="adobo", + masking_strategy=HashMaskingStrategy.name, + secret_type=SecretType.salt, + ) + cache_secret(secret, request_id) + + masked = masker.mask([datetime(2000, 1, 1)], request_id) + assert expected == masked + clear_cache_secrets(request_id) diff --git a/tests/ops/service/masking/strategy/test_masking_strategy_hmac.py b/tests/ops/service/masking/strategy/test_masking_strategy_hmac.py index 6b54c251136..9d22bd3d3bf 100644 --- a/tests/ops/service/masking/strategy/test_masking_strategy_hmac.py +++ b/tests/ops/service/masking/strategy/test_masking_strategy_hmac.py @@ -1,3 +1,5 @@ +from datetime import datetime + from fides.api.ops.schemas.masking.masking_configuration import HmacMaskingConfiguration from fides.api.ops.schemas.masking.masking_secrets import MaskingSecretCache, SecretType from fides.api.ops.service.masking.strategy.masking_strategy_hmac import ( @@ -124,3 +126,49 @@ def test_mask_arguments_null(): masked = masker.mask(None, request_id) assert expected == masked clear_cache_secrets(request_id) + + +def test_mask_arguments_null_list(): + configuration = HmacMaskingConfiguration() + masker = HmacMaskingStrategy(configuration) + expected = [None] + + secret_key = MaskingSecretCache[str]( + secret="test_key", + masking_strategy=HmacMaskingStrategy.name, + secret_type=SecretType.key, + ) + cache_secret(secret_key, request_id) + secret_salt = MaskingSecretCache[str]( + secret="test_salt", + masking_strategy=HmacMaskingStrategy.name, + secret_type=SecretType.salt, + ) + cache_secret(secret_salt, request_id) + + masked = masker.mask([None], request_id) + assert expected == masked + clear_cache_secrets(request_id) + + +def test_mask_arguments_date(): + configuration = HmacMaskingConfiguration() + masker = HmacMaskingStrategy(configuration) + expected = ["68cecd9f5bf6c4788da1513d64867b83eca1f7a260be104cb7a437dc863fc917"] + + secret_key = MaskingSecretCache[str]( + secret="test_key", + masking_strategy=HmacMaskingStrategy.name, + secret_type=SecretType.key, + ) + cache_secret(secret_key, request_id) + secret_salt = MaskingSecretCache[str]( + secret="test_salt", + masking_strategy=HmacMaskingStrategy.name, + secret_type=SecretType.salt, + ) + cache_secret(secret_salt, request_id) + + masked = masker.mask([datetime(2000, 1, 1)], request_id) + assert expected == masked + clear_cache_secrets(request_id)