From 6c52fd0e46c1a1d2b7f631af1c1e393c19a69a06 Mon Sep 17 00:00:00 2001 From: Tom Weininger Date: Fri, 25 Mar 2022 14:49:38 +0100 Subject: [PATCH] Validate L7Rule value and cookie name Adds validations in L7 rule and session cookie APIs in order to prevent authenticated and authorized users to inject code into HAProxy configuration. CR and LF (\r and \n) are no longer allowed in L7 rule keys and values. The session persistence cookie names must follow the rules described in https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie. Story: 2008994 Task: 44859 Change-Id: Ic370e9edc3fb5548e9cf0d66b85df66e01c41e79 (cherry picked from commit 3cf866dbc096ff2b6136d1e06da3f668f6ffeaaa) (cherry picked from commit 133ec4763db7889179ee09b897f503eb68ccd42e) (cherry picked from commit c30aa9df228548f30ff7609ee8f8f3cb36b5f89d) --- octavia/api/v2/types/l7rule.py | 13 +++++++--- octavia/api/v2/types/pool.py | 14 +++++++--- .../tests/unit/api/v2/types/test_l7rule.py | 26 ++++++++++++++++++- octavia/tests/unit/api/v2/types/test_pool.py | 10 +++++++ ...-and-session-cookies-cb88f3f1b90171f9.yaml | 9 +++++++ 5 files changed, 63 insertions(+), 9 deletions(-) create mode 100644 releasenotes/notes/fixed-API-validation-for-L7-rules-and-session-cookies-cb88f3f1b90171f9.yaml diff --git a/octavia/api/v2/types/l7rule.py b/octavia/api/v2/types/l7rule.py index 7c095f01bf..fbdddb7181 100644 --- a/octavia/api/v2/types/l7rule.py +++ b/octavia/api/v2/types/l7rule.py @@ -69,8 +69,11 @@ class L7RulePOST(BaseL7Type): compare_type = wtypes.wsattr( wtypes.Enum(str, *constants.SUPPORTED_L7RULE_COMPARE_TYPES), mandatory=True) - key = wtypes.wsattr(wtypes.StringType(max_length=255)) - value = wtypes.wsattr(wtypes.StringType(max_length=255), mandatory=True) + key = wtypes.wsattr(wtypes.StringType(max_length=255, + pattern=r'^[^\r\n]*$')) + value = wtypes.wsattr(wtypes.StringType(max_length=255, + pattern=r'^[^\r\n]*$'), + mandatory=True) invert = wtypes.wsattr(bool, default=False) admin_state_up = wtypes.wsattr(bool, default=True) # TODO(johnsom) Remove after deprecation (R series) @@ -90,8 +93,10 @@ class L7RulePUT(BaseL7Type): compare_type = wtypes.wsattr( wtypes.Enum(str, *constants.SUPPORTED_L7RULE_COMPARE_TYPES)) - key = wtypes.wsattr(wtypes.StringType(max_length=255)) - value = wtypes.wsattr(wtypes.StringType(max_length=255)) + key = wtypes.wsattr(wtypes.StringType(max_length=255, + pattern=r'^[^\r\n]*$')) + value = wtypes.wsattr(wtypes.StringType(max_length=255, + pattern=r'^[^\r\n]*$')) invert = wtypes.wsattr(bool) admin_state_up = wtypes.wsattr(bool) tags = wtypes.wsattr(wtypes.ArrayType(wtypes.StringType(max_length=255))) diff --git a/octavia/api/v2/types/pool.py b/octavia/api/v2/types/pool.py index f808fd534d..f3abead7f4 100644 --- a/octavia/api/v2/types/pool.py +++ b/octavia/api/v2/types/pool.py @@ -33,8 +33,11 @@ class SessionPersistencePOST(types.BaseType): """Defines mandatory and optional attributes of a POST request.""" type = wtypes.wsattr(wtypes.Enum(str, *constants.SUPPORTED_SP_TYPES), mandatory=True) - cookie_name = wtypes.wsattr(wtypes.StringType(max_length=255), - default=None) + # pattern of invalid characters is based on + # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie + cookie_name = wtypes.wsattr(wtypes.StringType( + max_length=255, pattern=r'^[^\s,;\\]+$'), + default=None) persistence_timeout = wtypes.wsattr(wtypes.IntegerType(), default=None) persistence_granularity = wtypes.wsattr(types.IPAddressType(), default=None) @@ -43,8 +46,11 @@ class SessionPersistencePOST(types.BaseType): class SessionPersistencePUT(types.BaseType): """Defines attributes that are acceptable of a PUT request.""" type = wtypes.wsattr(wtypes.Enum(str, *constants.SUPPORTED_SP_TYPES)) - cookie_name = wtypes.wsattr(wtypes.StringType(max_length=255), - default=None) + # pattern of invalid characters is based on + # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie + cookie_name = wtypes.wsattr(wtypes.StringType( + max_length=255, pattern=r'^[^\s,;\\]+$'), + default=None) persistence_timeout = wtypes.wsattr(wtypes.IntegerType(), default=None) persistence_granularity = wtypes.wsattr(types.IPAddressType(), default=None) diff --git a/octavia/tests/unit/api/v2/types/test_l7rule.py b/octavia/tests/unit/api/v2/types/test_l7rule.py index 2cf84e344d..1fecd2f0e2 100644 --- a/octavia/tests/unit/api/v2/types/test_l7rule.py +++ b/octavia/tests/unit/api/v2/types/test_l7rule.py @@ -67,12 +67,26 @@ class TestL7RulePOST(base.BaseTypesTest): body) def test_invalid_value(self): - body = {"type": "notvalid", + body = {"type": constants.L7RULE_TYPE_PATH, "compare_type": constants.L7RULE_COMPARE_TYPE_STARTS_WITH, "value": 123} self.assertRaises(exc.InvalidInput, wsme_json.fromjson, self._type, body) + def test_invalid_value_whitespace(self): + body = {"type": constants.L7RULE_TYPE_PATH, + "compare_type": constants.L7RULE_COMPARE_TYPE_STARTS_WITH, + "value": "12\n3"} + self.assertRaises(exc.InvalidInput, wsme_json.fromjson, self._type, + body) + + def test_invalid_key_whitespace(self): + body = {"type": constants.L7RULE_TYPE_PATH, + "compare_type": constants.L7RULE_COMPARE_TYPE_STARTS_WITH, + "key": "12\n3"} + self.assertRaises(exc.InvalidInput, wsme_json.fromjson, self._type, + body) + def test_invalid_invert(self): body = {"type": constants.L7RULE_TYPE_PATH, "compare_type": constants.L7RULE_COMPARE_TYPE_STARTS_WITH, @@ -139,6 +153,16 @@ class TestL7RulePUT(base.BaseTypesTest): self.assertRaises(exc.InvalidInput, wsme_json.fromjson, self._type, body) + def test_invalid_value_linefeed(self): + body = {"value": "12\n3"} + self.assertRaises(exc.InvalidInput, wsme_json.fromjson, self._type, + body) + + def test_invalid_key_linefeed(self): + body = {"key": "12\n3"} + self.assertRaises(exc.InvalidInput, wsme_json.fromjson, self._type, + body) + def test_invalid_invert(self): body = {"invert": "notvalid"} self.assertRaises(ValueError, wsme_json.fromjson, self._type, diff --git a/octavia/tests/unit/api/v2/types/test_pool.py b/octavia/tests/unit/api/v2/types/test_pool.py index f661b2ffa9..0007ae7a86 100644 --- a/octavia/tests/unit/api/v2/types/test_pool.py +++ b/octavia/tests/unit/api/v2/types/test_pool.py @@ -210,7 +210,17 @@ class TestSessionPersistencePOST(base.BaseTypesTest, TestSessionPersistence): self.assertRaises(exc.InvalidInput, wsme_json.fromjson, self._type, body) + def test_invalid_app_cookie_name(self): + body = {"cookie_name": "cookie,monster"} + self.assertRaises(exc.InvalidInput, wsme_json.fromjson, self._type, + body) + class TestSessionPersistencePUT(base.BaseTypesTest, TestSessionPersistence): _type = pool_type.SessionPersistencePUT + + def test_invalid_app_cookie_name(self): + body = {"cookie_name": "cookie\nmonster"} + self.assertRaises(exc.InvalidInput, wsme_json.fromjson, self._type, + body) diff --git a/releasenotes/notes/fixed-API-validation-for-L7-rules-and-session-cookies-cb88f3f1b90171f9.yaml b/releasenotes/notes/fixed-API-validation-for-L7-rules-and-session-cookies-cb88f3f1b90171f9.yaml new file mode 100644 index 0000000000..9a473553fb --- /dev/null +++ b/releasenotes/notes/fixed-API-validation-for-L7-rules-and-session-cookies-cb88f3f1b90171f9.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + Fixed validations in L7 rule and session cookie APIs in order to prevent + authenticated and authorized users to inject code into HAProxy + configuration. CR and LF (\r and \n) are no longer allowed in L7 rule + keys and values. The session persistence cookie names must follow the rules + described in + https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie.