diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9041981db5..fdd89307d9 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -312,6 +312,21 @@ Added Contributed by @Kami. +* Add new ``api.auth_cookie_secure`` and ``api.auth_cookie_same_site`` config options which + specify values which are set for ``secure`` and ``SameSite`` attribute for the auth cookie + we set when authenticating via token / api key in query parameter value (e.g. via st2web). + + For security reasons, ``api.auth_cookie_secure`` defaults to ``True``. This should only be + changed to ``False`` if you have a valid reason to not run StackStorm behind HTTPs proxy. + + Default value for ``api.auth_cookie_same_site`` is ``lax``. If you want to disable this + functionality so it behaves the same as in the previous releases, you can set that option + to ``None``. + + #5248 + + Contributed by @Kami. + * Mask secrets in output of an action execution in the API if the action has an output schema defined and one or more output parameters are marked as secret. #5250 diff --git a/conf/st2.conf.sample b/conf/st2.conf.sample index 9009cd0199..5dd450c2ad 100644 --- a/conf/st2.conf.sample +++ b/conf/st2.conf.sample @@ -38,6 +38,11 @@ workflows_pool_size = 40 [api] # List of origins allowed for api, auth and stream allow_origin = http://127.0.0.1:3000 # comma separated list allowed here. +# SameSite attribute value for the auth-token cookie we set on successful authentication from st2web. If you don't have a specific reason (e.g. supporting old browsers) we recommend you set this value to strict. Setting it to "unset" will default to the behavior in previous releases and not set this SameSite header value. +# Valid values: strict, lax, none, unset +auth_cookie_same_site = lax +# True if secure flag should be set for "auth-token" cookie which is set on successful authentication via st2web. You should only set this to False if you have a good reason to not run and access StackStorm behind https proxy. +auth_cookie_secure = True # None debug = False # StackStorm API server host @@ -142,6 +147,7 @@ ssl = False # ca_certs file contains a set of concatenated CA certificates, which are used to validate certificates passed from MongoDB. ssl_ca_certs = None # Specifies whether a certificate is required from the other side of the connection, and whether it will be validated if provided +# Valid values: none, optional, required ssl_cert_reqs = None # Certificate file used to identify the localconnection ssl_certfile = None @@ -151,7 +157,7 @@ ssl_keyfile = None ssl_match_hostname = True # username for db login username = None -# Compression level when compressors is set to zlib. Valid calues are -1 to 9. Defaults to 6. +# Compression level when compressors is set to zlib. Valid values are -1 to 9. Defaults to 6. zlib_compression_level = [exporter] @@ -195,7 +201,8 @@ redirect_stderr = False [messaging] # URL of all the nodes in a messaging service cluster. cluster_urls = # comma separated list allowed here. -# Compression algorithm to use for compressing the payloads which are sent over the message bus. Valid values include: zstd, lzma, bz2, gzip. Defaults to no compression. +# Compression algorithm to use for compressing the payloads which are sent over the message bus. Defaults to no compression. +# Valid values: zstd, lzma, bz2, gzip, None compression = None # How many times should we retry connection before failing. connection_retries = 10 @@ -208,6 +215,7 @@ ssl = False # ca_certs file contains a set of concatenated CA certificates, which are used to validate certificates passed from RabbitMQ. ssl_ca_certs = None # Specifies whether a certificate is required from the other side of the connection, and whether it will be validated if provided. +# Valid values: none, optional, required ssl_cert_reqs = None # Certificate file used to identify the local connection (client). ssl_certfile = None diff --git a/st2api/st2api/app.py b/st2api/st2api/app.py index 0495338000..ffb65ff313 100644 --- a/st2api/st2api/app.py +++ b/st2api/st2api/app.py @@ -28,6 +28,7 @@ from st2common.constants.system import VERSION_STRING from st2common.service_setup import setup as common_setup from st2common.util import spec_loader +from st2api.validation import validate_auth_cookie_is_correctly_configured from st2api.validation import validate_rbac_is_correctly_configured LOG = logging.getLogger(__name__) @@ -66,6 +67,7 @@ def setup_app(config=None): ) # Additional pre-run time checks + validate_auth_cookie_is_correctly_configured() validate_rbac_is_correctly_configured() router = Router( diff --git a/st2api/st2api/cmd/api.py b/st2api/st2api/cmd/api.py index 12c903f088..b80f6f4bed 100644 --- a/st2api/st2api/cmd/api.py +++ b/st2api/st2api/cmd/api.py @@ -37,6 +37,7 @@ config.register_opts(ignore_errors=True) from st2api import app +from st2api.validation import validate_auth_cookie_is_correctly_configured from st2api.validation import validate_rbac_is_correctly_configured __all__ = ["main"] @@ -68,6 +69,7 @@ def _setup(): ) # Additional pre-run time checks + validate_auth_cookie_is_correctly_configured() validate_rbac_is_correctly_configured() diff --git a/st2api/st2api/validation.py b/st2api/st2api/validation.py index 42120c57bf..f513094a98 100644 --- a/st2api/st2api/validation.py +++ b/st2api/st2api/validation.py @@ -15,10 +15,58 @@ from oslo_config import cfg -__all__ = ["validate_rbac_is_correctly_configured"] +from webob import cookies +__all__ = [ + "validate_auth_cookie_is_correctly_configured", + "validate_rbac_is_correctly_configured", +] -def validate_rbac_is_correctly_configured(): + +def validate_auth_cookie_is_correctly_configured() -> bool: + """ + Function which verifies that SameCookie config option value is correctly configured. + + This method should be called in the api init phase so we catch any misconfiguration issues + before startup. + """ + if cfg.CONF.api.auth_cookie_same_site not in ["strict", "lax", "none", "unset"]: + raise ValueError( + 'Got invalid value "%s" (type %s) for cfg.CONF.api.auth_cookie_same_site config ' + "option. Valid values are: strict, lax, none, unset." + % ( + cfg.CONF.api.auth_cookie_same_site, + type(cfg.CONF.api.auth_cookie_same_site), + ) + ) + + # Now we try to make a dummy cookie to verify all the options are configured correctly. Some + # Options are mutually exclusive - e.g. SameSite none and Secure false. + try: + # NOTE: none and unset don't mean the same thing - unset implies not setting this attribute + # (backward compatibility) and none implies setting this attribute value to none + same_site = cfg.CONF.api.auth_cookie_same_site + + kwargs = {} + if same_site != "unset": + kwargs["samesite"] = same_site + + cookies.make_cookie( + "test_cookie", + "dummyvalue", + httponly=True, + secure=cfg.CONF.api.auth_cookie_secure, + **kwargs, + ) + except Exception as e: + raise ValueError( + "Failed to validate api.auth_cookie config options: %s" % (str(e)) + ) + + return True + + +def validate_rbac_is_correctly_configured() -> bool: """ Function which verifies that RBAC is correctly set up and configured. """ diff --git a/st2api/tests/unit/controllers/v1/test_auth.py b/st2api/tests/unit/controllers/v1/test_auth.py index 8d3c499788..7fad0d816f 100644 --- a/st2api/tests/unit/controllers/v1/test_auth.py +++ b/st2api/tests/unit/controllers/v1/test_auth.py @@ -18,6 +18,7 @@ import bson import mock +from oslo_config import cfg from st2tests.api import FunctionalTest from st2common.util import date as date_utils @@ -69,6 +70,63 @@ def test_token_validation_token_in_query_params(self): self.assertIn("application/json", response.headers["content-type"]) self.assertEqual(response.status_int, 200) + @mock.patch.object( + Token, + "get", + mock.Mock( + return_value=TokenDB(id=OBJ_ID, user=USER, token=TOKEN, expiry=FUTURE) + ), + ) + @mock.patch.object(User, "get_by_name", mock.Mock(return_value=USER_DB)) + def test_token_validation_token_in_query_params_auth_cookie_is_set(self): + response = self.app.get( + "/v1/actions?x-auth-token=%s" % (TOKEN), expect_errors=False + ) + self.assertIn("application/json", response.headers["content-type"]) + self.assertEqual(response.status_int, 200) + self.assertTrue("Set-Cookie" in response.headers) + self.assertTrue("HttpOnly" in response.headers["Set-Cookie"]) + + # Also test same cookie values + secure + valid_values = ["strict", "lax", "none", "unset"] + + for value in valid_values: + cfg.CONF.set_override( + group="api", name="auth_cookie_same_site", override=value + ) + cfg.CONF.set_override(group="api", name="auth_cookie_secure", override=True) + + response = self.app.get( + "/v1/actions?x-auth-token=%s" % (TOKEN), expect_errors=False + ) + self.assertIn("application/json", response.headers["content-type"]) + self.assertEqual(response.status_int, 200) + self.assertTrue("Set-Cookie" in response.headers) + self.assertTrue("HttpOnly" in response.headers["Set-Cookie"]) + + if value == "unset": + self.assertFalse("SameSite" in response.headers["Set-Cookie"]) + else: + self.assertTrue( + "SameSite=%s" % (value) in response.headers["Set-Cookie"] + ) + + self.assertTrue("secure" in response.headers["Set-Cookie"]) + + # SameSite=Lax, Secure=False + cfg.CONF.set_override(group="api", name="auth_cookie_same_site", override="lax") + cfg.CONF.set_override(group="api", name="auth_cookie_secure", override=False) + + response = self.app.get( + "/v1/actions?x-auth-token=%s" % (TOKEN), expect_errors=False + ) + self.assertIn("application/json", response.headers["content-type"]) + self.assertEqual(response.status_int, 200) + self.assertTrue("Set-Cookie" in response.headers) + self.assertTrue("HttpOnly" in response.headers["Set-Cookie"]) + self.assertTrue("SameSite=lax" in response.headers["Set-Cookie"]) + self.assertTrue("secure" not in response.headers["Set-Cookie"]) + @mock.patch.object( Token, "get", diff --git a/st2api/tests/unit/test_validation_utils.py b/st2api/tests/unit/test_validation_utils.py index bad17b22a5..b5c939221c 100644 --- a/st2api/tests/unit/test_validation_utils.py +++ b/st2api/tests/unit/test_validation_utils.py @@ -16,6 +16,7 @@ import unittest2 from oslo_config import cfg +from st2api.validation import validate_auth_cookie_is_correctly_configured from st2api.validation import validate_rbac_is_correctly_configured from st2tests import config as tests_config @@ -27,6 +28,49 @@ def setUp(self): super(ValidationUtilsTestCase, self).setUp() tests_config.parse_args() + def test_validate_auth_cookie_is_correctly_configured_success(self): + valid_values = [ + "strict", + "lax", + "none", + "unset", + ] + + cfg.CONF.set_override(group="api", name="auth_cookie_secure", override=True) + + for value in valid_values: + cfg.CONF.set_override( + group="api", name="auth_cookie_same_site", override=value + ) + self.assertTrue(validate_auth_cookie_is_correctly_configured()) + + def test_validate_auth_cookie_is_correctly_configured_error(self): + invalid_values = ["strictx", "laxx", "nonex", "invalid"] + + for value in invalid_values: + cfg.CONF.set_override( + group="api", name="auth_cookie_same_site", override=value + ) + + expected_msg = "Valid values are: strict, lax, none, unset" + self.assertRaisesRegexp( + ValueError, expected_msg, validate_auth_cookie_is_correctly_configured + ) + + # SameSite=none + Secure=false is not compatible + cfg.CONF.set_override( + group="api", name="auth_cookie_same_site", override="none" + ) + cfg.CONF.set_override(group="api", name="auth_cookie_secure", override=False) + + expected_msg = ( + r"Failed to validate api.auth_cookie config options: Incompatible cookie attributes: " + "when the samesite equals 'none', then the secure must be True" + ) + self.assertRaisesRegexp( + ValueError, expected_msg, validate_auth_cookie_is_correctly_configured + ) + def test_validate_rbac_is_correctly_configured_succcess(self): result = validate_rbac_is_correctly_configured() self.assertTrue(result) diff --git a/st2common/st2common/config.py b/st2common/st2common/config.py index 8ad2f5d1e8..59cb9d01ab 100644 --- a/st2common/st2common/config.py +++ b/st2common/st2common/config.py @@ -254,7 +254,7 @@ def register_opts(ignore_errors=False): cfg.IntOpt( "zlib_compression_level", default="", - help="Compression level when compressors is set to zlib. Valid calues are -1 to 9. " + help="Compression level when compressors is set to zlib. Valid values are -1 to 9. " "Defaults to 6.", ), ] @@ -321,9 +321,9 @@ def register_opts(ignore_errors=False): cfg.StrOpt( "compression", default=None, + choices=["zstd", "lzma", "bz2", "gzip", None], help="Compression algorithm to use for compressing the payloads which are sent over " - "the message bus. Valid values include: zstd, lzma, bz2, gzip. Defaults to no " - "compression.", + "the message bus. Defaults to no compression.", ), ] @@ -373,6 +373,23 @@ def register_opts(ignore_errors=False): default=True, help="True to mask secrets in the API responses", ), + cfg.BoolOpt( + "auth_cookie_secure", + default=True, + help='True if secure flag should be set for "auth-token" cookie which is set on ' + "successful authentication via st2web. You should only set this to False if you have " + "a good reason to not run and access StackStorm behind https proxy.", + ), + cfg.StrOpt( + "auth_cookie_same_site", + default="lax", + choices=["strict", "lax", "none", "unset"], + help="SameSite attribute value for the " + "auth-token cookie we set on successful authentication from st2web. If you " + "don't have a specific reason (e.g. supporting old browsers) we recommend you " + 'set this value to strict. Setting it to "unset" will default to the behavior ' + "in previous releases and not set this SameSite header value.", + ), ] do_register_opts(api_opts, "api", ignore_errors) diff --git a/st2common/st2common/router.py b/st2common/st2common/router.py index acbb6cc5f0..ff4a9866f5 100644 --- a/st2common/st2common/router.py +++ b/st2common/st2common/router.py @@ -383,11 +383,22 @@ def __call__(self, req): max_age = ( auth_resp.expiry - date_utils.get_datetime_utc_now() ) + # NOTE: unset and none don't mean the same thing - unset implies + # not setting this attribute at all (backward compatibility) and + # none implies setting this attribute value to none + same_site = cfg.CONF.api.auth_cookie_same_site + + kwargs = {} + if same_site != "unset": + kwargs["samesite"] = same_site + cookie_token = cookies.make_cookie( definition["x-set-cookie"], token, max_age=max_age, httponly=True, + secure=cfg.CONF.api.auth_cookie_secure, + **kwargs, ) break diff --git a/st2tests/st2tests/config.py b/st2tests/st2tests/config.py index 0f44fa02a6..c4ae0cfcd1 100644 --- a/st2tests/st2tests/config.py +++ b/st2tests/st2tests/config.py @@ -111,6 +111,11 @@ def _override_api_opts(): override=["http://127.0.0.1:3000", "http://dev"], group="api", ) + CONF.set_override( + name="auth_cookie_secure", + override=False, + group="api", + ) def _override_keyvalue_opts(): diff --git a/tools/config_gen.py b/tools/config_gen.py index fc92195e9c..68a514ee74 100755 --- a/tools/config_gen.py +++ b/tools/config_gen.py @@ -175,6 +175,14 @@ def _print_options(opt_group, options): value = opt.default print(("# %s" % opt.help).strip()) + + if isinstance(opt, cfg.StrOpt) and opt.type.choices: + if isinstance(opt.type.choices, list): + valid_values = ", ".join([str(x) for x in opt.type.choices]) + else: + valid_values = opt.type.choices + print("# Valid values: %s" % (valid_values)) + print(("%s = %s" % (opt.name, value)).strip())