From 37ff757e877c9d1011dd499c67a179d41b535d71 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Thu, 19 Sep 2024 15:26:31 -0500 Subject: [PATCH 01/15] mongo: rename connection kwargs from ssl* to tls* --- st2common/st2common/config.py | 12 +++-- st2common/st2common/models/db/__init__.py | 40 +++++++++-------- st2common/tests/unit/test_db.py | 55 ++++++++++++++--------- 3 files changed, 62 insertions(+), 45 deletions(-) diff --git a/st2common/st2common/config.py b/st2common/st2common/config.py index f7e9cdc302..83b2c172ce 100644 --- a/st2common/st2common/config.py +++ b/st2common/st2common/config.py @@ -206,8 +206,12 @@ def register_opts(ignore_errors=False): help="Backoff multiplier (seconds).", ), cfg.BoolOpt( - "ssl", default=False, help="Create the connection to mongodb using SSL" + "ssl", # TODO: replace with "tls" + default=False, + help="Create the connection to mongodb using SSL", ), + # TODO: replace ssl_keyfile and ssl_certfile with tlsCertificateFile + # (see comment in st2common.models.db._get_ssl_kwargs) cfg.StrOpt( "ssl_keyfile", default=None, @@ -219,20 +223,20 @@ def register_opts(ignore_errors=False): help="Certificate file used to identify the localconnection", ), cfg.StrOpt( - "ssl_cert_reqs", + "ssl_cert_reqs", # TODO: replace with BoolOpt "tlsAllowInvalidCertificates" default=None, choices=["none", "optional", "required"], help="Specifies whether a certificate is required from the other side of the " "connection, and whether it will be validated if provided", ), cfg.StrOpt( - "ssl_ca_certs", + "ssl_ca_certs", # TODO: replace with "tlsCAFile" default=None, help="ca_certs file contains a set of concatenated CA certificates, which are " "used to validate certificates passed from MongoDB.", ), cfg.BoolOpt( - "ssl_match_hostname", + "ssl_match_hostname", # TODO: replace with "tlsAllowInvalidHostnames" default=True, help="If True and `ssl_cert_reqs` is not None, enables hostname verification", ), diff --git a/st2common/st2common/models/db/__init__.py b/st2common/st2common/models/db/__init__.py index 1cecf3a247..6346bc16f0 100644 --- a/st2common/st2common/models/db/__init__.py +++ b/st2common/st2common/models/db/__init__.py @@ -48,7 +48,6 @@ import copy import importlib import traceback -import ssl as ssl_lib import six from oslo_config import cfg @@ -444,34 +443,37 @@ def _get_ssl_kwargs( ): # NOTE: In pymongo 3.9.0 some of the ssl related arguments have been renamed - # https://api.mongodb.com/python/current/changelog.html#changes-in-version-3-9-0 - # Old names still work, but we should eventually update to new argument names. + # Old names stop working in pymongo 4, so we need to migrate now: + # https://pymongo.readthedocs.io/en/stable/migrate-to-pymongo4.html#renamed-uri-options ssl_kwargs = { - "ssl": ssl, + "tls": ssl, } + # TODO: replace ssl_keyfile and ssl_certfile with tlsCertificateFile per pymongo: + # > Instead of using ssl_certfile and ssl_keyfile to specify the certificate + # > and private key files respectively, use tlsCertificateKeyFile to pass a + # > single file containing both the client certificate and the private key. + # The tlsCertificateFile switch will be user-facing as files must be combined. if ssl_keyfile: - ssl_kwargs["ssl"] = True + ssl_kwargs["tls"] = True ssl_kwargs["ssl_keyfile"] = ssl_keyfile if ssl_certfile: - ssl_kwargs["ssl"] = True + ssl_kwargs["tls"] = True ssl_kwargs["ssl_certfile"] = ssl_certfile if ssl_cert_reqs: - if ssl_cert_reqs == "none": - ssl_cert_reqs = ssl_lib.CERT_NONE - elif ssl_cert_reqs == "optional": - ssl_cert_reqs = ssl_lib.CERT_OPTIONAL - elif ssl_cert_reqs == "required": - ssl_cert_reqs = ssl_lib.CERT_REQUIRED - ssl_kwargs["ssl_cert_reqs"] = ssl_cert_reqs + # possible values: none, optional, required + # ssl lib docs say 'optional' is the same as 'required' for clients: + # https://docs.python.org/3/library/ssl.html#ssl.CERT_OPTIONAL + ssl_kwargs["tlsAllowInvalidCertificates"] = ssl_cert_reqs == "none" if ssl_ca_certs: - ssl_kwargs["ssl"] = True - ssl_kwargs["ssl_ca_certs"] = ssl_ca_certs + ssl_kwargs["tls"] = True + ssl_kwargs["tlsCAFile"] = ssl_ca_certs if authentication_mechanism: - ssl_kwargs["ssl"] = True + ssl_kwargs["tls"] = True ssl_kwargs["authentication_mechanism"] = authentication_mechanism - if ssl_kwargs.get("ssl", False): - # pass in ssl_match_hostname only if ssl is True. The right default value - # for ssl_match_hostname in almost all cases is True. - ssl_kwargs["ssl_match_hostname"] = ssl_match_hostname + if ssl_kwargs.get("tls", False): + # pass in tlsAllowInvalidHostname only if ssl is True. The right default value + # for tlsAllowInvalidHostname in almost all cases is False. + ssl_kwargs["tlsAllowInvalidHostnames"] = not ssl_match_hostname return ssl_kwargs diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index ed6b88649d..acaf46abd2 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -21,7 +21,6 @@ monkey_patch() -import ssl import time import jsonschema @@ -229,19 +228,19 @@ def test_network_level_compression(self): def test_get_ssl_kwargs(self): # 1. No SSL kwargs provided ssl_kwargs = _get_ssl_kwargs() - self.assertEqual(ssl_kwargs, {"ssl": False}) + self.assertEqual(ssl_kwargs, {"tls": False}) # 2. ssl kwarg provided ssl_kwargs = _get_ssl_kwargs(ssl=True) - self.assertEqual(ssl_kwargs, {"ssl": True, "ssl_match_hostname": True}) + self.assertEqual(ssl_kwargs, {"tls": True, "tlsAllowInvalidHostnames": False}) # 2. authentication_mechanism kwarg provided ssl_kwargs = _get_ssl_kwargs(authentication_mechanism="MONGODB-X509") self.assertEqual( ssl_kwargs, { - "ssl": True, - "ssl_match_hostname": True, + "tls": True, + "tlsAllowInvalidHostnames": False, "authentication_mechanism": "MONGODB-X509", }, ) @@ -250,21 +249,33 @@ def test_get_ssl_kwargs(self): ssl_kwargs = _get_ssl_kwargs(ssl_keyfile="/tmp/keyfile") self.assertEqual( ssl_kwargs, - {"ssl": True, "ssl_keyfile": "/tmp/keyfile", "ssl_match_hostname": True}, + { + "tls": True, + "ssl_keyfile": "/tmp/keyfile", + "tlsAllowInvalidHostnames": False, + }, ) # 4. ssl_certfile provided ssl_kwargs = _get_ssl_kwargs(ssl_certfile="/tmp/certfile") self.assertEqual( ssl_kwargs, - {"ssl": True, "ssl_certfile": "/tmp/certfile", "ssl_match_hostname": True}, + { + "tls": True, + "ssl_certfile": "/tmp/certfile", + "tlsAllowInvalidHostnames": False, + }, ) # 5. ssl_ca_certs provided ssl_kwargs = _get_ssl_kwargs(ssl_ca_certs="/tmp/ca_certs") self.assertEqual( ssl_kwargs, - {"ssl": True, "ssl_ca_certs": "/tmp/ca_certs", "ssl_match_hostname": True}, + { + "tls": True, + "tlsCAFile": "/tmp/ca_certs", + "tlsAllowInvalidHostnames": False, + }, ) # 6. ssl_ca_certs and ssl_cert_reqs combinations @@ -272,10 +283,10 @@ def test_get_ssl_kwargs(self): self.assertEqual( ssl_kwargs, { - "ssl": True, - "ssl_ca_certs": "/tmp/ca_certs", - "ssl_cert_reqs": ssl.CERT_NONE, - "ssl_match_hostname": True, + "tls": True, + "tlsCAFile": "/tmp/ca_certs", + "tlsAllowInvalidCertificates": True, + "tlsAllowInvalidHostnames": False, }, ) @@ -285,10 +296,10 @@ def test_get_ssl_kwargs(self): self.assertEqual( ssl_kwargs, { - "ssl": True, - "ssl_ca_certs": "/tmp/ca_certs", - "ssl_cert_reqs": ssl.CERT_OPTIONAL, - "ssl_match_hostname": True, + "tls": True, + "tlsCAFile": "/tmp/ca_certs", + "tlsAllowInvalidCertificates": False, + "tlsAllowInvalidHostnames": False, }, ) @@ -298,10 +309,10 @@ def test_get_ssl_kwargs(self): self.assertEqual( ssl_kwargs, { - "ssl": True, - "ssl_ca_certs": "/tmp/ca_certs", - "ssl_cert_reqs": ssl.CERT_REQUIRED, - "ssl_match_hostname": True, + "tls": True, + "tlsCAFile": "/tmp/ca_certs", + "tlsAllowInvalidCertificates": False, + "tlsAllowInvalidHostnames": False, }, ) @@ -330,8 +341,8 @@ def test_db_setup(self, mock_mongoengine): "password": "password", "tz_aware": True, "authentication_mechanism": "MONGODB-X509", - "ssl": True, - "ssl_match_hostname": True, + "tls": True, + "tlsAllowInvalidHostnames": False, "connectTimeoutMS": 3000, "serverSelectionTimeoutMS": 3000, }, From 362286882c0a8c6496a3ff6b54f58f4d3933050b Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 20 Sep 2024 14:41:59 -0500 Subject: [PATCH 02/15] Rename database.ssl opt to database.tls --- contrib/packs/actions/pack_mgmt/unload.py | 2 +- st2common/st2common/config.py | 5 +++-- st2common/st2common/database_setup.py | 2 +- st2common/st2common/models/db/__init__.py | 18 +++++++++--------- st2common/st2common/persistence/cleanup.py | 4 ++-- st2common/st2common/persistence/db_init.py | 4 ++-- st2common/tests/unit/test_db.py | 8 ++++---- .../st2reactor/container/sensor_wrapper.py | 2 +- 8 files changed, 23 insertions(+), 22 deletions(-) diff --git a/contrib/packs/actions/pack_mgmt/unload.py b/contrib/packs/actions/pack_mgmt/unload.py index b26182329d..46016bf446 100644 --- a/contrib/packs/actions/pack_mgmt/unload.py +++ b/contrib/packs/actions/pack_mgmt/unload.py @@ -59,7 +59,7 @@ def initialize(self): cfg.CONF.database.port, username=username, password=password, - ssl=cfg.CONF.database.ssl, + tls=cfg.CONF.database.tls, ssl_keyfile=cfg.CONF.database.ssl_keyfile, ssl_certfile=cfg.CONF.database.ssl_certfile, ssl_cert_reqs=cfg.CONF.database.ssl_cert_reqs, diff --git a/st2common/st2common/config.py b/st2common/st2common/config.py index 83b2c172ce..93b39489b8 100644 --- a/st2common/st2common/config.py +++ b/st2common/st2common/config.py @@ -206,9 +206,10 @@ def register_opts(ignore_errors=False): help="Backoff multiplier (seconds).", ), cfg.BoolOpt( - "ssl", # TODO: replace with "tls" + "tls", + deprecated_name="ssl", default=False, - help="Create the connection to mongodb using SSL", + help="Create the connection to mongodb using TLS.", ), # TODO: replace ssl_keyfile and ssl_certfile with tlsCertificateFile # (see comment in st2common.models.db._get_ssl_kwargs) diff --git a/st2common/st2common/database_setup.py b/st2common/st2common/database_setup.py index 2e2e7d2a17..7eac34619d 100644 --- a/st2common/st2common/database_setup.py +++ b/st2common/st2common/database_setup.py @@ -36,7 +36,7 @@ def db_config(): "db_port": cfg.CONF.database.port, "username": username, "password": password, - "ssl": cfg.CONF.database.ssl, + "tls": cfg.CONF.database.tls, "ssl_keyfile": cfg.CONF.database.ssl_keyfile, "ssl_certfile": cfg.CONF.database.ssl_certfile, "ssl_cert_reqs": cfg.CONF.database.ssl_cert_reqs, diff --git a/st2common/st2common/models/db/__init__.py b/st2common/st2common/models/db/__init__.py index 6346bc16f0..cb608a9d66 100644 --- a/st2common/st2common/models/db/__init__.py +++ b/st2common/st2common/models/db/__init__.py @@ -126,7 +126,7 @@ def _db_connect( db_port, username=None, password=None, - ssl=False, + tls=False, ssl_keyfile=None, ssl_certfile=None, ssl_cert_reqs=None, @@ -161,7 +161,7 @@ def _db_connect( ) ssl_kwargs = _get_ssl_kwargs( - ssl=ssl, + tls=tls, ssl_keyfile=ssl_keyfile, ssl_certfile=ssl_certfile, ssl_cert_reqs=ssl_cert_reqs, @@ -230,7 +230,7 @@ def db_setup( username=None, password=None, ensure_indexes=True, - ssl=False, + tls=False, ssl_keyfile=None, ssl_certfile=None, ssl_cert_reqs=None, @@ -245,7 +245,7 @@ def db_setup( db_port, username=username, password=password, - ssl=ssl, + tls=tls, ssl_keyfile=ssl_keyfile, ssl_certfile=ssl_certfile, ssl_cert_reqs=ssl_cert_reqs, @@ -396,7 +396,7 @@ def db_cleanup( db_port, username=None, password=None, - ssl=False, + tls=False, ssl_keyfile=None, ssl_certfile=None, ssl_cert_reqs=None, @@ -411,7 +411,7 @@ def db_cleanup( db_port, username=username, password=password, - ssl=ssl, + tls=tls, ssl_keyfile=ssl_keyfile, ssl_certfile=ssl_certfile, ssl_cert_reqs=ssl_cert_reqs, @@ -433,7 +433,7 @@ def db_cleanup( def _get_ssl_kwargs( - ssl=False, + tls=False, ssl_keyfile=None, ssl_certfile=None, ssl_cert_reqs=None, @@ -446,7 +446,7 @@ def _get_ssl_kwargs( # Old names stop working in pymongo 4, so we need to migrate now: # https://pymongo.readthedocs.io/en/stable/migrate-to-pymongo4.html#renamed-uri-options ssl_kwargs = { - "tls": ssl, + "tls": tls, } # TODO: replace ssl_keyfile and ssl_certfile with tlsCertificateFile per pymongo: # > Instead of using ssl_certfile and ssl_keyfile to specify the certificate @@ -471,7 +471,7 @@ def _get_ssl_kwargs( ssl_kwargs["tls"] = True ssl_kwargs["authentication_mechanism"] = authentication_mechanism if ssl_kwargs.get("tls", False): - # pass in tlsAllowInvalidHostname only if ssl is True. The right default value + # pass in tlsAllowInvalidHostname only if tls is True. The right default value # for tlsAllowInvalidHostname in almost all cases is False. ssl_kwargs["tlsAllowInvalidHostnames"] = not ssl_match_hostname return ssl_kwargs diff --git a/st2common/st2common/persistence/cleanup.py b/st2common/st2common/persistence/cleanup.py index 06c48dec86..1a63a1a92c 100644 --- a/st2common/st2common/persistence/cleanup.py +++ b/st2common/st2common/persistence/cleanup.py @@ -44,7 +44,7 @@ def db_cleanup_with_retry( db_port, username=None, password=None, - ssl=False, + tls=False, ssl_keyfile=None, ssl_certfile=None, ssl_cert_reqs=None, @@ -62,7 +62,7 @@ def db_cleanup_with_retry( db_port, username=username, password=password, - ssl=ssl, + tls=tls, ssl_keyfile=ssl_keyfile, ssl_certfile=ssl_certfile, ssl_cert_reqs=ssl_cert_reqs, diff --git a/st2common/st2common/persistence/db_init.py b/st2common/st2common/persistence/db_init.py index c7c0bd9f83..5b7bc5a1f8 100644 --- a/st2common/st2common/persistence/db_init.py +++ b/st2common/st2common/persistence/db_init.py @@ -65,7 +65,7 @@ def db_setup_with_retry( username=None, password=None, ensure_indexes=True, - ssl=False, + tls=False, ssl_keyfile=None, ssl_certfile=None, ssl_cert_reqs=None, @@ -84,7 +84,7 @@ def db_setup_with_retry( username=username, password=password, ensure_indexes=ensure_indexes, - ssl=ssl, + tls=tls, ssl_keyfile=ssl_keyfile, ssl_certfile=ssl_certfile, ssl_cert_reqs=ssl_cert_reqs, diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index acaf46abd2..63d71de677 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -230,8 +230,8 @@ def test_get_ssl_kwargs(self): ssl_kwargs = _get_ssl_kwargs() self.assertEqual(ssl_kwargs, {"tls": False}) - # 2. ssl kwarg provided - ssl_kwargs = _get_ssl_kwargs(ssl=True) + # 2. tls kwarg provided + ssl_kwargs = _get_ssl_kwargs(tls=True) self.assertEqual(ssl_kwargs, {"tls": True, "tlsAllowInvalidHostnames": False}) # 2. authentication_mechanism kwarg provided @@ -547,7 +547,7 @@ def test_db_connect_server_selection_timeout_ssl_on_non_ssl_listener(self): db_name=db_name, db_host=db_host, db_port=db_port, - ssl=True, + tls=True, ensure_indexes=False, ) end = time.time() @@ -566,7 +566,7 @@ def test_db_connect_server_selection_timeout_ssl_on_non_ssl_listener(self): db_name=db_name, db_host=db_host, db_port=db_port, - ssl=True, + tls=True, ensure_indexes=False, ) end = time.time() diff --git a/st2reactor/st2reactor/container/sensor_wrapper.py b/st2reactor/st2reactor/container/sensor_wrapper.py index 6b1975bc53..f8663993f9 100644 --- a/st2reactor/st2reactor/container/sensor_wrapper.py +++ b/st2reactor/st2reactor/container/sensor_wrapper.py @@ -231,7 +231,7 @@ def __init__( username=username, password=password, ensure_indexes=db_ensure_indexes, - ssl=cfg.CONF.database.ssl, + tls=cfg.CONF.database.tls, ssl_keyfile=cfg.CONF.database.ssl_keyfile, ssl_certfile=cfg.CONF.database.ssl_certfile, ssl_cert_reqs=cfg.CONF.database.ssl_cert_reqs, From b62b8eddc0239c507b5341620376fb4c379afec8 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 20 Sep 2024 14:47:17 -0500 Subject: [PATCH 03/15] refactor: rename ssl_kwargs to tls_kwargs (db funcs) --- st2common/st2common/models/db/__init__.py | 32 +++++++++--------- st2common/tests/unit/test_db.py | 40 +++++++++++------------ 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/st2common/st2common/models/db/__init__.py b/st2common/st2common/models/db/__init__.py index cb608a9d66..19846d6ecb 100644 --- a/st2common/st2common/models/db/__init__.py +++ b/st2common/st2common/models/db/__init__.py @@ -160,7 +160,7 @@ def _db_connect( % (db_name, host_string, str(username_string)) ) - ssl_kwargs = _get_ssl_kwargs( + tls_kwargs = _get_tls_kwargs( tls=tls, ssl_keyfile=ssl_keyfile, ssl_certfile=ssl_certfile, @@ -193,7 +193,7 @@ def _db_connect( password=password, connectTimeoutMS=connection_timeout, serverSelectionTimeoutMS=connection_timeout, - **ssl_kwargs, + **tls_kwargs, **compressor_kwargs, ) @@ -432,7 +432,7 @@ def db_cleanup( return connection -def _get_ssl_kwargs( +def _get_tls_kwargs( tls=False, ssl_keyfile=None, ssl_certfile=None, @@ -445,7 +445,7 @@ def _get_ssl_kwargs( # https://api.mongodb.com/python/current/changelog.html#changes-in-version-3-9-0 # Old names stop working in pymongo 4, so we need to migrate now: # https://pymongo.readthedocs.io/en/stable/migrate-to-pymongo4.html#renamed-uri-options - ssl_kwargs = { + tls_kwargs = { "tls": tls, } # TODO: replace ssl_keyfile and ssl_certfile with tlsCertificateFile per pymongo: @@ -454,27 +454,27 @@ def _get_ssl_kwargs( # > single file containing both the client certificate and the private key. # The tlsCertificateFile switch will be user-facing as files must be combined. if ssl_keyfile: - ssl_kwargs["tls"] = True - ssl_kwargs["ssl_keyfile"] = ssl_keyfile + tls_kwargs["tls"] = True + tls_kwargs["ssl_keyfile"] = ssl_keyfile if ssl_certfile: - ssl_kwargs["tls"] = True - ssl_kwargs["ssl_certfile"] = ssl_certfile + tls_kwargs["tls"] = True + tls_kwargs["ssl_certfile"] = ssl_certfile if ssl_cert_reqs: # possible values: none, optional, required # ssl lib docs say 'optional' is the same as 'required' for clients: # https://docs.python.org/3/library/ssl.html#ssl.CERT_OPTIONAL - ssl_kwargs["tlsAllowInvalidCertificates"] = ssl_cert_reqs == "none" + tls_kwargs["tlsAllowInvalidCertificates"] = ssl_cert_reqs == "none" if ssl_ca_certs: - ssl_kwargs["tls"] = True - ssl_kwargs["tlsCAFile"] = ssl_ca_certs + tls_kwargs["tls"] = True + tls_kwargs["tlsCAFile"] = ssl_ca_certs if authentication_mechanism: - ssl_kwargs["tls"] = True - ssl_kwargs["authentication_mechanism"] = authentication_mechanism - if ssl_kwargs.get("tls", False): + tls_kwargs["tls"] = True + tls_kwargs["authentication_mechanism"] = authentication_mechanism + if tls_kwargs.get("tls", False): # pass in tlsAllowInvalidHostname only if tls is True. The right default value # for tlsAllowInvalidHostname in almost all cases is False. - ssl_kwargs["tlsAllowInvalidHostnames"] = not ssl_match_hostname - return ssl_kwargs + tls_kwargs["tlsAllowInvalidHostnames"] = not ssl_match_hostname + return tls_kwargs class MongoDBAccess(object): diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 63d71de677..4e74065ffa 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -37,7 +37,7 @@ from st2common.util import schema as util_schema from st2common.util import reference from st2common.models.db import db_setup -from st2common.models.db import _get_ssl_kwargs +from st2common.models.db import _get_tls_kwargs from st2common.util import date as date_utils from st2common.exceptions.db import StackStormDBObjectNotFoundError from st2common.models.db.trigger import TriggerTypeDB, TriggerDB, TriggerInstanceDB @@ -225,19 +225,19 @@ def test_network_level_compression(self): self.assertTrue("compressors=['zlib']" in str(connection)) self.assertTrue("zlibcompressionlevel=9" in str(connection)) - def test_get_ssl_kwargs(self): + def test_get_tls_kwargs(self): # 1. No SSL kwargs provided - ssl_kwargs = _get_ssl_kwargs() - self.assertEqual(ssl_kwargs, {"tls": False}) + tls_kwargs = _get_tls_kwargs() + self.assertEqual(tls_kwargs, {"tls": False}) # 2. tls kwarg provided - ssl_kwargs = _get_ssl_kwargs(tls=True) - self.assertEqual(ssl_kwargs, {"tls": True, "tlsAllowInvalidHostnames": False}) + tls_kwargs = _get_tls_kwargs(tls=True) + self.assertEqual(tls_kwargs, {"tls": True, "tlsAllowInvalidHostnames": False}) # 2. authentication_mechanism kwarg provided - ssl_kwargs = _get_ssl_kwargs(authentication_mechanism="MONGODB-X509") + tls_kwargs = _get_tls_kwargs(authentication_mechanism="MONGODB-X509") self.assertEqual( - ssl_kwargs, + tls_kwargs, { "tls": True, "tlsAllowInvalidHostnames": False, @@ -246,9 +246,9 @@ def test_get_ssl_kwargs(self): ) # 3. ssl_keyfile provided - ssl_kwargs = _get_ssl_kwargs(ssl_keyfile="/tmp/keyfile") + tls_kwargs = _get_tls_kwargs(ssl_keyfile="/tmp/keyfile") self.assertEqual( - ssl_kwargs, + tls_kwargs, { "tls": True, "ssl_keyfile": "/tmp/keyfile", @@ -257,9 +257,9 @@ def test_get_ssl_kwargs(self): ) # 4. ssl_certfile provided - ssl_kwargs = _get_ssl_kwargs(ssl_certfile="/tmp/certfile") + tls_kwargs = _get_tls_kwargs(ssl_certfile="/tmp/certfile") self.assertEqual( - ssl_kwargs, + tls_kwargs, { "tls": True, "ssl_certfile": "/tmp/certfile", @@ -268,9 +268,9 @@ def test_get_ssl_kwargs(self): ) # 5. ssl_ca_certs provided - ssl_kwargs = _get_ssl_kwargs(ssl_ca_certs="/tmp/ca_certs") + tls_kwargs = _get_tls_kwargs(ssl_ca_certs="/tmp/ca_certs") self.assertEqual( - ssl_kwargs, + tls_kwargs, { "tls": True, "tlsCAFile": "/tmp/ca_certs", @@ -279,9 +279,9 @@ def test_get_ssl_kwargs(self): ) # 6. ssl_ca_certs and ssl_cert_reqs combinations - ssl_kwargs = _get_ssl_kwargs(ssl_ca_certs="/tmp/ca_certs", ssl_cert_reqs="none") + tls_kwargs = _get_tls_kwargs(ssl_ca_certs="/tmp/ca_certs", ssl_cert_reqs="none") self.assertEqual( - ssl_kwargs, + tls_kwargs, { "tls": True, "tlsCAFile": "/tmp/ca_certs", @@ -290,11 +290,11 @@ def test_get_ssl_kwargs(self): }, ) - ssl_kwargs = _get_ssl_kwargs( + tls_kwargs = _get_tls_kwargs( ssl_ca_certs="/tmp/ca_certs", ssl_cert_reqs="optional" ) self.assertEqual( - ssl_kwargs, + tls_kwargs, { "tls": True, "tlsCAFile": "/tmp/ca_certs", @@ -303,11 +303,11 @@ def test_get_ssl_kwargs(self): }, ) - ssl_kwargs = _get_ssl_kwargs( + tls_kwargs = _get_tls_kwargs( ssl_ca_certs="/tmp/ca_certs", ssl_cert_reqs="required" ) self.assertEqual( - ssl_kwargs, + tls_kwargs, { "tls": True, "tlsCAFile": "/tmp/ca_certs", From dc62d113a19ac6eac42c8370a7101e0456f69bf2 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 20 Sep 2024 15:19:38 -0500 Subject: [PATCH 04/15] add database.tls_certificate_key_file* opts to replace ssl_keyfile/ssl_certfile pymongo 4 will ignore the ssl_keyfile/ssl_certfile options. For consistency in st2.conf, this uses snake_case not the mongo camelCase option name. This also adds tls_certificate_key_file_password. We did not support ssl_pem_passphrase before, so there was nothing to migrate. --- contrib/packs/actions/pack_mgmt/unload.py | 6 +- st2common/st2common/config.py | 36 ++++++++++- st2common/st2common/database_setup.py | 6 +- st2common/st2common/models/db/__init__.py | 59 +++++++++++-------- st2common/st2common/persistence/cleanup.py | 12 ++-- st2common/st2common/persistence/db_init.py | 12 ++-- st2common/tests/unit/test_db.py | 30 ++++++++-- .../st2reactor/container/sensor_wrapper.py | 6 +- 8 files changed, 121 insertions(+), 46 deletions(-) diff --git a/contrib/packs/actions/pack_mgmt/unload.py b/contrib/packs/actions/pack_mgmt/unload.py index 46016bf446..1a6c96f154 100644 --- a/contrib/packs/actions/pack_mgmt/unload.py +++ b/contrib/packs/actions/pack_mgmt/unload.py @@ -60,8 +60,10 @@ def initialize(self): username=username, password=password, tls=cfg.CONF.database.tls, - ssl_keyfile=cfg.CONF.database.ssl_keyfile, - ssl_certfile=cfg.CONF.database.ssl_certfile, + tls_certificate_key_file=cfg.CONF.database.tls_certificate_key_file, + tls_certificate_key_file_password=cfg.CONF.database.tls_certificate_key_file_password, + ssl_keyfile=cfg.CONF.database.ssl_keyfile, # deprecated / unused + ssl_certfile=cfg.CONF.database.ssl_certfile, # deprecated / unused ssl_cert_reqs=cfg.CONF.database.ssl_cert_reqs, ssl_ca_certs=cfg.CONF.database.ssl_ca_certs, authentication_mechanism=cfg.CONF.database.authentication_mechanism, diff --git a/st2common/st2common/config.py b/st2common/st2common/config.py index 93b39489b8..4196a75c9f 100644 --- a/st2common/st2common/config.py +++ b/st2common/st2common/config.py @@ -211,17 +211,49 @@ def register_opts(ignore_errors=False): default=False, help="Create the connection to mongodb using TLS.", ), - # TODO: replace ssl_keyfile and ssl_certfile with tlsCertificateFile - # (see comment in st2common.models.db._get_ssl_kwargs) + cfg.StrOpt( + "tls_certificate_key_file", + default=None, + help=( + "Client certificate used to identify the local connection against MongoDB. " + "The certificate file must contain one or both of private key and certificate. " + "Supplying separate files for private key (ssl_keyfile) and certificate (ssl_certfile) " + "is no longer supported. " + "If encrypted, pass the password or passphrase in tls_certificate_key_file_password." + ), + ), + cfg.StrOpt( + "tls_certificate_key_file_password", + default=None, + help=( + "The password or passphrase to decrypt the file in tls_certificate_key_file. " + "Only set this if tls_certificate_key_file is encrypted." + ), + secret=True, + ), cfg.StrOpt( "ssl_keyfile", default=None, help="Private keyfile used to identify the local connection against MongoDB.", + deprecated_for_removal=True, + deprecated_reason=( + "Use tls_certificate_key_file with a path to a file containing " + "the concatenation of the files from ssl_keyfile and ssl_certfile. " + "This option is ignored by pymongo." + ), + deprecated_since="3.9.0", ), cfg.StrOpt( "ssl_certfile", default=None, help="Certificate file used to identify the localconnection", + deprecated_for_removal=True, + deprecated_reason=( + "Use tls_certificate_key_file with a path to a file containing " + "the concatenation of the files from ssl_keyfile and ssl_certfile. " + "This option is ignored by pymongo. " + ), + deprecated_since="3.9.0", ), cfg.StrOpt( "ssl_cert_reqs", # TODO: replace with BoolOpt "tlsAllowInvalidCertificates" diff --git a/st2common/st2common/database_setup.py b/st2common/st2common/database_setup.py index 7eac34619d..314453e069 100644 --- a/st2common/st2common/database_setup.py +++ b/st2common/st2common/database_setup.py @@ -37,8 +37,10 @@ def db_config(): "username": username, "password": password, "tls": cfg.CONF.database.tls, - "ssl_keyfile": cfg.CONF.database.ssl_keyfile, - "ssl_certfile": cfg.CONF.database.ssl_certfile, + "tls_certificate_key_file": cfg.CONF.database.tls_certificate_key_file, + "tls_certificate_key_file_password": cfg.CONF.database.tls_certificate_key_file_password, + "ssl_keyfile": cfg.CONF.database.ssl_keyfile, # deprecated / unused + "ssl_certfile": cfg.CONF.database.ssl_certfile, # deprecated / unused "ssl_cert_reqs": cfg.CONF.database.ssl_cert_reqs, "ssl_ca_certs": cfg.CONF.database.ssl_ca_certs, "authentication_mechanism": cfg.CONF.database.authentication_mechanism, diff --git a/st2common/st2common/models/db/__init__.py b/st2common/st2common/models/db/__init__.py index 19846d6ecb..c13c825567 100644 --- a/st2common/st2common/models/db/__init__.py +++ b/st2common/st2common/models/db/__init__.py @@ -127,8 +127,10 @@ def _db_connect( username=None, password=None, tls=False, - ssl_keyfile=None, - ssl_certfile=None, + tls_certificate_key_file=None, + tls_certificate_key_file_password=None, + ssl_keyfile=None, # deprecated / unused + ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, ssl_ca_certs=None, authentication_mechanism=None, @@ -162,8 +164,10 @@ def _db_connect( tls_kwargs = _get_tls_kwargs( tls=tls, - ssl_keyfile=ssl_keyfile, - ssl_certfile=ssl_certfile, + tls_certificate_key_file=tls_certificate_key_file, + tls_certificate_key_file_password=tls_certificate_key_file_password, + ssl_keyfile=ssl_keyfile, # deprecated / unused + ssl_certfile=ssl_certfile, # deprecated / unused ssl_cert_reqs=ssl_cert_reqs, ssl_ca_certs=ssl_ca_certs, authentication_mechanism=authentication_mechanism, @@ -231,8 +235,10 @@ def db_setup( password=None, ensure_indexes=True, tls=False, - ssl_keyfile=None, - ssl_certfile=None, + tls_certificate_key_file=None, + tls_certificate_key_file_password=None, + ssl_keyfile=None, # deprecated / unused + ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, ssl_ca_certs=None, authentication_mechanism=None, @@ -246,8 +252,10 @@ def db_setup( username=username, password=password, tls=tls, - ssl_keyfile=ssl_keyfile, - ssl_certfile=ssl_certfile, + tls_certificate_key_file=tls_certificate_key_file, + tls_certificate_key_file_password=tls_certificate_key_file_password, + ssl_keyfile=ssl_keyfile, # deprecated / unused + ssl_certfile=ssl_certfile, # deprecated / unused ssl_cert_reqs=ssl_cert_reqs, ssl_ca_certs=ssl_ca_certs, authentication_mechanism=authentication_mechanism, @@ -397,8 +405,10 @@ def db_cleanup( username=None, password=None, tls=False, - ssl_keyfile=None, - ssl_certfile=None, + tls_certificate_key_file=None, + tls_certificate_key_file_password=None, + ssl_keyfile=None, # deprecated / unused + ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, ssl_ca_certs=None, authentication_mechanism=None, @@ -412,8 +422,10 @@ def db_cleanup( username=username, password=password, tls=tls, - ssl_keyfile=ssl_keyfile, - ssl_certfile=ssl_certfile, + tls_certificate_key_file=tls_certificate_key_file, + tls_certificate_key_file_password=tls_certificate_key_file_password, + ssl_keyfile=ssl_keyfile, # deprecated / unused + ssl_certfile=ssl_certfile, # deprecated / unused ssl_cert_reqs=ssl_cert_reqs, ssl_ca_certs=ssl_ca_certs, authentication_mechanism=authentication_mechanism, @@ -434,8 +446,10 @@ def db_cleanup( def _get_tls_kwargs( tls=False, - ssl_keyfile=None, - ssl_certfile=None, + tls_certificate_key_file=None, + tls_certificate_key_file_password=None, + ssl_keyfile=None, # deprecated / unused + ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, ssl_ca_certs=None, authentication_mechanism=None, @@ -448,17 +462,14 @@ def _get_tls_kwargs( tls_kwargs = { "tls": tls, } - # TODO: replace ssl_keyfile and ssl_certfile with tlsCertificateFile per pymongo: - # > Instead of using ssl_certfile and ssl_keyfile to specify the certificate - # > and private key files respectively, use tlsCertificateKeyFile to pass a - # > single file containing both the client certificate and the private key. - # The tlsCertificateFile switch will be user-facing as files must be combined. - if ssl_keyfile: + # pymongo 4 ignores ssl_keyfile and ssl_certfile, so we do not need to pass them on. + if tls_certificate_key_file: tls_kwargs["tls"] = True - tls_kwargs["ssl_keyfile"] = ssl_keyfile - if ssl_certfile: - tls_kwargs["tls"] = True - tls_kwargs["ssl_certfile"] = ssl_certfile + tls_kwargs["tlsCertificateKeyFile"] = tls_certificate_key_file + if tls_certificate_key_file_password: + tls_kwargs[ + "tlsCertificateKeyFilePassword" + ] = tls_certificate_key_file_password if ssl_cert_reqs: # possible values: none, optional, required # ssl lib docs say 'optional' is the same as 'required' for clients: diff --git a/st2common/st2common/persistence/cleanup.py b/st2common/st2common/persistence/cleanup.py index 1a63a1a92c..455780ca15 100644 --- a/st2common/st2common/persistence/cleanup.py +++ b/st2common/st2common/persistence/cleanup.py @@ -45,8 +45,10 @@ def db_cleanup_with_retry( username=None, password=None, tls=False, - ssl_keyfile=None, - ssl_certfile=None, + tls_certificate_key_file=None, + tls_certificate_key_file_password=None, + ssl_keyfile=None, # deprecated / unused + ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, ssl_ca_certs=None, authentication_mechanism=None, @@ -63,8 +65,10 @@ def db_cleanup_with_retry( username=username, password=password, tls=tls, - ssl_keyfile=ssl_keyfile, - ssl_certfile=ssl_certfile, + tls_certificate_key_file=tls_certificate_key_file, + tls_certificate_key_file_password=tls_certificate_key_file_password, + ssl_keyfile=ssl_keyfile, # deprecated / unused + ssl_certfile=ssl_certfile, # deprecated / unused ssl_cert_reqs=ssl_cert_reqs, ssl_ca_certs=ssl_ca_certs, authentication_mechanism=authentication_mechanism, diff --git a/st2common/st2common/persistence/db_init.py b/st2common/st2common/persistence/db_init.py index 5b7bc5a1f8..2b6098f611 100644 --- a/st2common/st2common/persistence/db_init.py +++ b/st2common/st2common/persistence/db_init.py @@ -66,8 +66,10 @@ def db_setup_with_retry( password=None, ensure_indexes=True, tls=False, - ssl_keyfile=None, - ssl_certfile=None, + tls_certificate_key_file=None, + tls_certificate_key_file_password=None, + ssl_keyfile=None, # deprecated / unused + ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, ssl_ca_certs=None, authentication_mechanism=None, @@ -85,8 +87,10 @@ def db_setup_with_retry( password=password, ensure_indexes=ensure_indexes, tls=tls, - ssl_keyfile=ssl_keyfile, - ssl_certfile=ssl_certfile, + tls_certificate_key_file=tls_certificate_key_file, + tls_certificate_key_file_password=tls_certificate_key_file_password, + ssl_keyfile=ssl_keyfile, # deprecated / unused + ssl_certfile=ssl_certfile, # deprecated / unused ssl_cert_reqs=ssl_cert_reqs, ssl_ca_certs=ssl_ca_certs, authentication_mechanism=authentication_mechanism, diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 4e74065ffa..f10acf1cf2 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -245,28 +245,46 @@ def test_get_tls_kwargs(self): }, ) - # 3. ssl_keyfile provided - tls_kwargs = _get_tls_kwargs(ssl_keyfile="/tmp/keyfile") + # 3. ssl_keyfile and ssl_certfile are ignored by pymongo so this does too. + tls_kwargs = _get_tls_kwargs(ssl_keyfile="/tmp/keyfile", ssl_certfile="/tmp/certfile") self.assertEqual( tls_kwargs, { "tls": True, - "ssl_keyfile": "/tmp/keyfile", "tlsAllowInvalidHostnames": False, }, ) - # 4. ssl_certfile provided - tls_kwargs = _get_tls_kwargs(ssl_certfile="/tmp/certfile") + # 4a. tls_certificate_key_file provided + tls_kwargs = _get_tls_kwargs(tls_certificate_key_file="/tmp/keyfile") self.assertEqual( tls_kwargs, { "tls": True, - "ssl_certfile": "/tmp/certfile", + "tlsCertificateKeyFile": "/tmp/keyfile", "tlsAllowInvalidHostnames": False, }, ) + # 4b. tls_certificate_key_file_password provided with tls_certificate_key_file + tls_kwargs = _get_tls_kwargs( + tls_certificate_key_file="/tmp/keyfile", + tls_certificate_key_file_password="pass", + ) + self.assertEqual( + tls_kwargs, + { + "tls": True, + "tlsCertificateKeyFile": "/tmp/keyfile", + "tlsCertificateKeyFilePassword": "pass", + "tlsAllowInvalidHostnames": False, + }, + ) + + # 4c. tls_certificate_key_file_password provided without tls_certificate_key_file + tls_kwargs = _get_tls_kwargs(tls_certificate_key_file_password="pass") + self.assertEqual(tls_kwargs, {"tls": False}) + # 5. ssl_ca_certs provided tls_kwargs = _get_tls_kwargs(ssl_ca_certs="/tmp/ca_certs") self.assertEqual( diff --git a/st2reactor/st2reactor/container/sensor_wrapper.py b/st2reactor/st2reactor/container/sensor_wrapper.py index f8663993f9..223d87ce70 100644 --- a/st2reactor/st2reactor/container/sensor_wrapper.py +++ b/st2reactor/st2reactor/container/sensor_wrapper.py @@ -232,8 +232,10 @@ def __init__( password=password, ensure_indexes=db_ensure_indexes, tls=cfg.CONF.database.tls, - ssl_keyfile=cfg.CONF.database.ssl_keyfile, - ssl_certfile=cfg.CONF.database.ssl_certfile, + tls_certificate_key_file=cfg.CONF.database.tls_certificate_key_file, + tls_certificate_key_file_password=cfg.CONF.database.tls_certificate_key_file_password, + ssl_keyfile=cfg.CONF.database.ssl_keyfile, # deprecated / unused + ssl_certfile=cfg.CONF.database.ssl_certfile, # deprecated / unused ssl_cert_reqs=cfg.CONF.database.ssl_cert_reqs, ssl_ca_certs=cfg.CONF.database.ssl_ca_certs, authentication_mechanism=cfg.CONF.database.authentication_mechanism, From 2094ff4e1717aa60990c3e5cbbedf217810070c7 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 20 Sep 2024 15:37:34 -0500 Subject: [PATCH 05/15] add database.tls_allow_invalid_certificates to replace ssl_cert_reqs This needed to be a different option (instead of just renaming) because the option type is changing from str+choices to a bool. For consistency in st2.conf, this uses snake_case not the mongo camelCase option name. --- contrib/packs/actions/pack_mgmt/unload.py | 3 ++- st2common/st2common/config.py | 26 ++++++++++++++++--- st2common/st2common/database_setup.py | 3 ++- st2common/st2common/models/db/__init__.py | 25 ++++++++++++------ st2common/st2common/persistence/cleanup.py | 6 +++-- st2common/st2common/persistence/db_init.py | 6 +++-- st2common/tests/unit/test_db.py | 25 ++++++++++++++++++ .../st2reactor/container/sensor_wrapper.py | 3 ++- 8 files changed, 79 insertions(+), 18 deletions(-) diff --git a/contrib/packs/actions/pack_mgmt/unload.py b/contrib/packs/actions/pack_mgmt/unload.py index 1a6c96f154..ed03b2af47 100644 --- a/contrib/packs/actions/pack_mgmt/unload.py +++ b/contrib/packs/actions/pack_mgmt/unload.py @@ -62,9 +62,10 @@ def initialize(self): tls=cfg.CONF.database.tls, tls_certificate_key_file=cfg.CONF.database.tls_certificate_key_file, tls_certificate_key_file_password=cfg.CONF.database.tls_certificate_key_file_password, + tls_allow_invalid_certificates=cfg.CONF.database.tls_allow_invalid_certificates, ssl_keyfile=cfg.CONF.database.ssl_keyfile, # deprecated / unused ssl_certfile=cfg.CONF.database.ssl_certfile, # deprecated / unused - ssl_cert_reqs=cfg.CONF.database.ssl_cert_reqs, + ssl_cert_reqs=cfg.CONF.database.ssl_cert_reqs, # deprecated ssl_ca_certs=cfg.CONF.database.ssl_ca_certs, authentication_mechanism=cfg.CONF.database.authentication_mechanism, ssl_match_hostname=cfg.CONF.database.ssl_match_hostname, diff --git a/st2common/st2common/config.py b/st2common/st2common/config.py index 4196a75c9f..cbcdf42745 100644 --- a/st2common/st2common/config.py +++ b/st2common/st2common/config.py @@ -255,12 +255,32 @@ def register_opts(ignore_errors=False): ), deprecated_since="3.9.0", ), + cfg.BoolOpt( + "tls_allow_invalid_certificates", + default=None, + sample_default=False, + help=( + "Specifies whether MongoDB is allowed to pass an invalid certificate. " + "This defaults to False to have security by default. " + "Only temporarily set to True if you need to debug the connection." + ), + ), cfg.StrOpt( - "ssl_cert_reqs", # TODO: replace with BoolOpt "tlsAllowInvalidCertificates" + "ssl_cert_reqs", default=None, choices=["none", "optional", "required"], - help="Specifies whether a certificate is required from the other side of the " - "connection, and whether it will be validated if provided", + help=( + "Specifies whether a certificate is required from the other side of the " + "connection, and whether it will be validated if provided" + ), + deprecated_for_removal=True, + deprecated_reason=( + "Use tls_allow_invalid_certificates with the following: " + "The 'optional' and 'required' values are equivalent to tls_allow_invalid_certificates=False. " + "The 'none' value is equivalent to tls_allow_invalid_certificates=True. " + "This option is a needlessly more complex version of tls_allow_invalid_certificates." + ), + deprecated_since="3.9.0", ), cfg.StrOpt( "ssl_ca_certs", # TODO: replace with "tlsCAFile" diff --git a/st2common/st2common/database_setup.py b/st2common/st2common/database_setup.py index 314453e069..917d3d7dd8 100644 --- a/st2common/st2common/database_setup.py +++ b/st2common/st2common/database_setup.py @@ -39,9 +39,10 @@ def db_config(): "tls": cfg.CONF.database.tls, "tls_certificate_key_file": cfg.CONF.database.tls_certificate_key_file, "tls_certificate_key_file_password": cfg.CONF.database.tls_certificate_key_file_password, + "tls_allow_invalid_certificates": cfg.CONF.database.tls_allow_invalid_certificates, "ssl_keyfile": cfg.CONF.database.ssl_keyfile, # deprecated / unused "ssl_certfile": cfg.CONF.database.ssl_certfile, # deprecated / unused - "ssl_cert_reqs": cfg.CONF.database.ssl_cert_reqs, + "ssl_cert_reqs": cfg.CONF.database.ssl_cert_reqs, # deprecated "ssl_ca_certs": cfg.CONF.database.ssl_ca_certs, "authentication_mechanism": cfg.CONF.database.authentication_mechanism, "ssl_match_hostname": cfg.CONF.database.ssl_match_hostname, diff --git a/st2common/st2common/models/db/__init__.py b/st2common/st2common/models/db/__init__.py index c13c825567..fea9965459 100644 --- a/st2common/st2common/models/db/__init__.py +++ b/st2common/st2common/models/db/__init__.py @@ -129,9 +129,10 @@ def _db_connect( tls=False, tls_certificate_key_file=None, tls_certificate_key_file_password=None, + tls_allow_invalid_certificates=None, ssl_keyfile=None, # deprecated / unused ssl_certfile=None, # deprecated / unused - ssl_cert_reqs=None, + ssl_cert_reqs=None, # deprecated ssl_ca_certs=None, authentication_mechanism=None, ssl_match_hostname=True, @@ -166,9 +167,10 @@ def _db_connect( tls=tls, tls_certificate_key_file=tls_certificate_key_file, tls_certificate_key_file_password=tls_certificate_key_file_password, + tls_allow_invalid_certificates=tls_allow_invalid_certificates, ssl_keyfile=ssl_keyfile, # deprecated / unused ssl_certfile=ssl_certfile, # deprecated / unused - ssl_cert_reqs=ssl_cert_reqs, + ssl_cert_reqs=ssl_cert_reqs, # deprecated ssl_ca_certs=ssl_ca_certs, authentication_mechanism=authentication_mechanism, ssl_match_hostname=ssl_match_hostname, @@ -237,9 +239,10 @@ def db_setup( tls=False, tls_certificate_key_file=None, tls_certificate_key_file_password=None, + tls_allow_invalid_certificates=None, ssl_keyfile=None, # deprecated / unused ssl_certfile=None, # deprecated / unused - ssl_cert_reqs=None, + ssl_cert_reqs=None, # deprecated ssl_ca_certs=None, authentication_mechanism=None, ssl_match_hostname=True, @@ -254,9 +257,10 @@ def db_setup( tls=tls, tls_certificate_key_file=tls_certificate_key_file, tls_certificate_key_file_password=tls_certificate_key_file_password, + tls_allow_invalid_certificates=tls_allow_invalid_certificates, ssl_keyfile=ssl_keyfile, # deprecated / unused ssl_certfile=ssl_certfile, # deprecated / unused - ssl_cert_reqs=ssl_cert_reqs, + ssl_cert_reqs=ssl_cert_reqs, # deprecated ssl_ca_certs=ssl_ca_certs, authentication_mechanism=authentication_mechanism, ssl_match_hostname=ssl_match_hostname, @@ -407,9 +411,10 @@ def db_cleanup( tls=False, tls_certificate_key_file=None, tls_certificate_key_file_password=None, + tls_allow_invalid_certificates=None, ssl_keyfile=None, # deprecated / unused ssl_certfile=None, # deprecated / unused - ssl_cert_reqs=None, + ssl_cert_reqs=None, # deprecated ssl_ca_certs=None, authentication_mechanism=None, ssl_match_hostname=True, @@ -424,9 +429,10 @@ def db_cleanup( tls=tls, tls_certificate_key_file=tls_certificate_key_file, tls_certificate_key_file_password=tls_certificate_key_file_password, + tls_allow_invalid_certificates=tls_allow_invalid_certificates, ssl_keyfile=ssl_keyfile, # deprecated / unused ssl_certfile=ssl_certfile, # deprecated / unused - ssl_cert_reqs=ssl_cert_reqs, + ssl_cert_reqs=ssl_cert_reqs, # deprecated ssl_ca_certs=ssl_ca_certs, authentication_mechanism=authentication_mechanism, ssl_match_hostname=ssl_match_hostname, @@ -448,9 +454,10 @@ def _get_tls_kwargs( tls=False, tls_certificate_key_file=None, tls_certificate_key_file_password=None, + tls_allow_invalid_certificates=None, ssl_keyfile=None, # deprecated / unused ssl_certfile=None, # deprecated / unused - ssl_cert_reqs=None, + ssl_cert_reqs=None, # deprecated ssl_ca_certs=None, authentication_mechanism=None, ssl_match_hostname=True, @@ -470,7 +477,9 @@ def _get_tls_kwargs( tls_kwargs[ "tlsCertificateKeyFilePassword" ] = tls_certificate_key_file_password - if ssl_cert_reqs: + if tls_allow_invalid_certificates is not None: + tls_kwargs["tlsAllowInvalidCertificates"] = tls_allow_invalid_certificates + elif ssl_cert_reqs: # fall back to old option # possible values: none, optional, required # ssl lib docs say 'optional' is the same as 'required' for clients: # https://docs.python.org/3/library/ssl.html#ssl.CERT_OPTIONAL diff --git a/st2common/st2common/persistence/cleanup.py b/st2common/st2common/persistence/cleanup.py index 455780ca15..46fcf815c7 100644 --- a/st2common/st2common/persistence/cleanup.py +++ b/st2common/st2common/persistence/cleanup.py @@ -47,9 +47,10 @@ def db_cleanup_with_retry( tls=False, tls_certificate_key_file=None, tls_certificate_key_file_password=None, + tls_allow_invalid_certificates=None, ssl_keyfile=None, # deprecated / unused ssl_certfile=None, # deprecated / unused - ssl_cert_reqs=None, + ssl_cert_reqs=None, # deprecated ssl_ca_certs=None, authentication_mechanism=None, ssl_match_hostname=True, @@ -67,9 +68,10 @@ def db_cleanup_with_retry( tls=tls, tls_certificate_key_file=tls_certificate_key_file, tls_certificate_key_file_password=tls_certificate_key_file_password, + tls_allow_invalid_certificates=tls_allow_invalid_certificates, ssl_keyfile=ssl_keyfile, # deprecated / unused ssl_certfile=ssl_certfile, # deprecated / unused - ssl_cert_reqs=ssl_cert_reqs, + ssl_cert_reqs=ssl_cert_reqs, # deprecated ssl_ca_certs=ssl_ca_certs, authentication_mechanism=authentication_mechanism, ssl_match_hostname=ssl_match_hostname, diff --git a/st2common/st2common/persistence/db_init.py b/st2common/st2common/persistence/db_init.py index 2b6098f611..6d4e1c80f5 100644 --- a/st2common/st2common/persistence/db_init.py +++ b/st2common/st2common/persistence/db_init.py @@ -68,9 +68,10 @@ def db_setup_with_retry( tls=False, tls_certificate_key_file=None, tls_certificate_key_file_password=None, + tls_allow_invalid_certificates=None, ssl_keyfile=None, # deprecated / unused ssl_certfile=None, # deprecated / unused - ssl_cert_reqs=None, + ssl_cert_reqs=None, # deprecated ssl_ca_certs=None, authentication_mechanism=None, ssl_match_hostname=True, @@ -89,9 +90,10 @@ def db_setup_with_retry( tls=tls, tls_certificate_key_file=tls_certificate_key_file, tls_certificate_key_file_password=tls_certificate_key_file_password, + tls_allow_invalid_certificates=tls_allow_invalid_certificates, ssl_keyfile=ssl_keyfile, # deprecated / unused ssl_certfile=ssl_certfile, # deprecated / unused - ssl_cert_reqs=ssl_cert_reqs, + ssl_cert_reqs=ssl_cert_reqs, # deprecated ssl_ca_certs=ssl_ca_certs, authentication_mechanism=authentication_mechanism, ssl_match_hostname=ssl_match_hostname, diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index f10acf1cf2..4613333dac 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -334,6 +334,31 @@ def test_get_tls_kwargs(self): }, ) + # 7. tls_allow_invalid_certificates provided (does not implicitly enable tls) + for allow_invalid in (True, False): + tls_kwargs = _get_tls_kwargs(tls_allow_invalid_certificates=allow_invalid) + self.assertEqual( + tls_kwargs, + { + "tls": False, + "tlsAllowInvalidCertificates": allow_invalid, + }, + ) + + # make sure ssl_cert_reqs is ignored if tls_allow_invalid_certificates is set + for ssl_cert_reqs in ("none", "optional", "required"): + tls_kwargs = _get_tls_kwargs( + ssl_cert_reqs=ssl_cert_reqs, + tls_allow_invalid_certificates=allow_invalid, + ) + self.assertEqual( + tls_kwargs, + { + "tls": False, + "tlsAllowInvalidCertificates": allow_invalid, + }, + ) + @mock.patch("st2common.models.db.mongoengine") def test_db_setup(self, mock_mongoengine): db_setup( diff --git a/st2reactor/st2reactor/container/sensor_wrapper.py b/st2reactor/st2reactor/container/sensor_wrapper.py index 223d87ce70..8ae1d04740 100644 --- a/st2reactor/st2reactor/container/sensor_wrapper.py +++ b/st2reactor/st2reactor/container/sensor_wrapper.py @@ -234,9 +234,10 @@ def __init__( tls=cfg.CONF.database.tls, tls_certificate_key_file=cfg.CONF.database.tls_certificate_key_file, tls_certificate_key_file_password=cfg.CONF.database.tls_certificate_key_file_password, + tls_allow_invalid_certificates=cfg.CONF.database.tls_allow_invalid_certificates, ssl_keyfile=cfg.CONF.database.ssl_keyfile, # deprecated / unused ssl_certfile=cfg.CONF.database.ssl_certfile, # deprecated / unused - ssl_cert_reqs=cfg.CONF.database.ssl_cert_reqs, + ssl_cert_reqs=cfg.CONF.database.ssl_cert_reqs, # deprecated ssl_ca_certs=cfg.CONF.database.ssl_ca_certs, authentication_mechanism=cfg.CONF.database.authentication_mechanism, ssl_match_hostname=cfg.CONF.database.ssl_match_hostname, From e610bdc5bd3223def7eb39bb74be89043308179b Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 20 Sep 2024 16:51:08 -0500 Subject: [PATCH 06/15] rename database.ssl_ca_certs opt to database.tls_ca_file For consistency in st2.conf, this uses snake_case not the mongo camelCase option name. --- contrib/packs/actions/pack_mgmt/unload.py | 2 +- st2common/st2common/config.py | 9 ++++++--- st2common/st2common/database_setup.py | 2 +- st2common/st2common/models/db/__init__.py | 18 +++++++++--------- st2common/st2common/persistence/cleanup.py | 4 ++-- st2common/st2common/persistence/db_init.py | 4 ++-- st2common/tests/unit/test_db.py | 12 ++++++------ .../st2reactor/container/sensor_wrapper.py | 2 +- 8 files changed, 28 insertions(+), 25 deletions(-) diff --git a/contrib/packs/actions/pack_mgmt/unload.py b/contrib/packs/actions/pack_mgmt/unload.py index ed03b2af47..c70663e4dc 100644 --- a/contrib/packs/actions/pack_mgmt/unload.py +++ b/contrib/packs/actions/pack_mgmt/unload.py @@ -63,10 +63,10 @@ def initialize(self): tls_certificate_key_file=cfg.CONF.database.tls_certificate_key_file, tls_certificate_key_file_password=cfg.CONF.database.tls_certificate_key_file_password, tls_allow_invalid_certificates=cfg.CONF.database.tls_allow_invalid_certificates, + tls_ca_file=cfg.CONF.database.tls_ca_file, ssl_keyfile=cfg.CONF.database.ssl_keyfile, # deprecated / unused ssl_certfile=cfg.CONF.database.ssl_certfile, # deprecated / unused ssl_cert_reqs=cfg.CONF.database.ssl_cert_reqs, # deprecated - ssl_ca_certs=cfg.CONF.database.ssl_ca_certs, authentication_mechanism=cfg.CONF.database.authentication_mechanism, ssl_match_hostname=cfg.CONF.database.ssl_match_hostname, ) diff --git a/st2common/st2common/config.py b/st2common/st2common/config.py index cbcdf42745..696a83cea5 100644 --- a/st2common/st2common/config.py +++ b/st2common/st2common/config.py @@ -283,10 +283,13 @@ def register_opts(ignore_errors=False): deprecated_since="3.9.0", ), cfg.StrOpt( - "ssl_ca_certs", # TODO: replace with "tlsCAFile" + "tls_ca_file", + deprecated_name="ssl_ca_certs", default=None, - help="ca_certs file contains a set of concatenated CA certificates, which are " - "used to validate certificates passed from MongoDB.", + help=( + "ca_certs file contains a set of concatenated CA certificates, which are " + "used to validate certificates passed from MongoDB." + ), ), cfg.BoolOpt( "ssl_match_hostname", # TODO: replace with "tlsAllowInvalidHostnames" diff --git a/st2common/st2common/database_setup.py b/st2common/st2common/database_setup.py index 917d3d7dd8..c0a0c2d5f9 100644 --- a/st2common/st2common/database_setup.py +++ b/st2common/st2common/database_setup.py @@ -40,10 +40,10 @@ def db_config(): "tls_certificate_key_file": cfg.CONF.database.tls_certificate_key_file, "tls_certificate_key_file_password": cfg.CONF.database.tls_certificate_key_file_password, "tls_allow_invalid_certificates": cfg.CONF.database.tls_allow_invalid_certificates, + "tls_ca_file": cfg.CONF.database.tls_ca_file, "ssl_keyfile": cfg.CONF.database.ssl_keyfile, # deprecated / unused "ssl_certfile": cfg.CONF.database.ssl_certfile, # deprecated / unused "ssl_cert_reqs": cfg.CONF.database.ssl_cert_reqs, # deprecated - "ssl_ca_certs": cfg.CONF.database.ssl_ca_certs, "authentication_mechanism": cfg.CONF.database.authentication_mechanism, "ssl_match_hostname": cfg.CONF.database.ssl_match_hostname, } diff --git a/st2common/st2common/models/db/__init__.py b/st2common/st2common/models/db/__init__.py index fea9965459..873a4ed2f0 100644 --- a/st2common/st2common/models/db/__init__.py +++ b/st2common/st2common/models/db/__init__.py @@ -130,10 +130,10 @@ def _db_connect( tls_certificate_key_file=None, tls_certificate_key_file_password=None, tls_allow_invalid_certificates=None, + tls_ca_file=None, ssl_keyfile=None, # deprecated / unused ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, # deprecated - ssl_ca_certs=None, authentication_mechanism=None, ssl_match_hostname=True, ): @@ -168,10 +168,10 @@ def _db_connect( tls_certificate_key_file=tls_certificate_key_file, tls_certificate_key_file_password=tls_certificate_key_file_password, tls_allow_invalid_certificates=tls_allow_invalid_certificates, + tls_ca_file=tls_ca_file, ssl_keyfile=ssl_keyfile, # deprecated / unused ssl_certfile=ssl_certfile, # deprecated / unused ssl_cert_reqs=ssl_cert_reqs, # deprecated - ssl_ca_certs=ssl_ca_certs, authentication_mechanism=authentication_mechanism, ssl_match_hostname=ssl_match_hostname, ) @@ -240,10 +240,10 @@ def db_setup( tls_certificate_key_file=None, tls_certificate_key_file_password=None, tls_allow_invalid_certificates=None, + tls_ca_file=None, ssl_keyfile=None, # deprecated / unused ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, # deprecated - ssl_ca_certs=None, authentication_mechanism=None, ssl_match_hostname=True, ): @@ -258,10 +258,10 @@ def db_setup( tls_certificate_key_file=tls_certificate_key_file, tls_certificate_key_file_password=tls_certificate_key_file_password, tls_allow_invalid_certificates=tls_allow_invalid_certificates, + tls_ca_file=tls_ca_file, ssl_keyfile=ssl_keyfile, # deprecated / unused ssl_certfile=ssl_certfile, # deprecated / unused ssl_cert_reqs=ssl_cert_reqs, # deprecated - ssl_ca_certs=ssl_ca_certs, authentication_mechanism=authentication_mechanism, ssl_match_hostname=ssl_match_hostname, ) @@ -412,10 +412,10 @@ def db_cleanup( tls_certificate_key_file=None, tls_certificate_key_file_password=None, tls_allow_invalid_certificates=None, + tls_ca_file=None, ssl_keyfile=None, # deprecated / unused ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, # deprecated - ssl_ca_certs=None, authentication_mechanism=None, ssl_match_hostname=True, ): @@ -430,10 +430,10 @@ def db_cleanup( tls_certificate_key_file=tls_certificate_key_file, tls_certificate_key_file_password=tls_certificate_key_file_password, tls_allow_invalid_certificates=tls_allow_invalid_certificates, + tls_ca_file=tls_ca_file, ssl_keyfile=ssl_keyfile, # deprecated / unused ssl_certfile=ssl_certfile, # deprecated / unused ssl_cert_reqs=ssl_cert_reqs, # deprecated - ssl_ca_certs=ssl_ca_certs, authentication_mechanism=authentication_mechanism, ssl_match_hostname=ssl_match_hostname, ) @@ -455,10 +455,10 @@ def _get_tls_kwargs( tls_certificate_key_file=None, tls_certificate_key_file_password=None, tls_allow_invalid_certificates=None, + tls_ca_file=None, ssl_keyfile=None, # deprecated / unused ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, # deprecated - ssl_ca_certs=None, authentication_mechanism=None, ssl_match_hostname=True, ): @@ -484,9 +484,9 @@ def _get_tls_kwargs( # ssl lib docs say 'optional' is the same as 'required' for clients: # https://docs.python.org/3/library/ssl.html#ssl.CERT_OPTIONAL tls_kwargs["tlsAllowInvalidCertificates"] = ssl_cert_reqs == "none" - if ssl_ca_certs: + if tls_ca_file: tls_kwargs["tls"] = True - tls_kwargs["tlsCAFile"] = ssl_ca_certs + tls_kwargs["tlsCAFile"] = tls_ca_file if authentication_mechanism: tls_kwargs["tls"] = True tls_kwargs["authentication_mechanism"] = authentication_mechanism diff --git a/st2common/st2common/persistence/cleanup.py b/st2common/st2common/persistence/cleanup.py index 46fcf815c7..d807c63b98 100644 --- a/st2common/st2common/persistence/cleanup.py +++ b/st2common/st2common/persistence/cleanup.py @@ -48,10 +48,10 @@ def db_cleanup_with_retry( tls_certificate_key_file=None, tls_certificate_key_file_password=None, tls_allow_invalid_certificates=None, + tls_ca_file=None, ssl_keyfile=None, # deprecated / unused ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, # deprecated - ssl_ca_certs=None, authentication_mechanism=None, ssl_match_hostname=True, ): @@ -69,10 +69,10 @@ def db_cleanup_with_retry( tls_certificate_key_file=tls_certificate_key_file, tls_certificate_key_file_password=tls_certificate_key_file_password, tls_allow_invalid_certificates=tls_allow_invalid_certificates, + tls_ca_file=tls_ca_file, ssl_keyfile=ssl_keyfile, # deprecated / unused ssl_certfile=ssl_certfile, # deprecated / unused ssl_cert_reqs=ssl_cert_reqs, # deprecated - ssl_ca_certs=ssl_ca_certs, authentication_mechanism=authentication_mechanism, ssl_match_hostname=ssl_match_hostname, ) diff --git a/st2common/st2common/persistence/db_init.py b/st2common/st2common/persistence/db_init.py index 6d4e1c80f5..62a14826f5 100644 --- a/st2common/st2common/persistence/db_init.py +++ b/st2common/st2common/persistence/db_init.py @@ -69,10 +69,10 @@ def db_setup_with_retry( tls_certificate_key_file=None, tls_certificate_key_file_password=None, tls_allow_invalid_certificates=None, + tls_ca_file=None, ssl_keyfile=None, # deprecated / unused ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, # deprecated - ssl_ca_certs=None, authentication_mechanism=None, ssl_match_hostname=True, ): @@ -91,10 +91,10 @@ def db_setup_with_retry( tls_certificate_key_file=tls_certificate_key_file, tls_certificate_key_file_password=tls_certificate_key_file_password, tls_allow_invalid_certificates=tls_allow_invalid_certificates, + tls_ca_file=tls_ca_file, ssl_keyfile=ssl_keyfile, # deprecated / unused ssl_certfile=ssl_certfile, # deprecated / unused ssl_cert_reqs=ssl_cert_reqs, # deprecated - ssl_ca_certs=ssl_ca_certs, authentication_mechanism=authentication_mechanism, ssl_match_hostname=ssl_match_hostname, ) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 4613333dac..03742aa003 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -285,8 +285,8 @@ def test_get_tls_kwargs(self): tls_kwargs = _get_tls_kwargs(tls_certificate_key_file_password="pass") self.assertEqual(tls_kwargs, {"tls": False}) - # 5. ssl_ca_certs provided - tls_kwargs = _get_tls_kwargs(ssl_ca_certs="/tmp/ca_certs") + # 5. tls_ca_file provided + tls_kwargs = _get_tls_kwargs(tls_ca_file="/tmp/ca_certs") self.assertEqual( tls_kwargs, { @@ -296,8 +296,8 @@ def test_get_tls_kwargs(self): }, ) - # 6. ssl_ca_certs and ssl_cert_reqs combinations - tls_kwargs = _get_tls_kwargs(ssl_ca_certs="/tmp/ca_certs", ssl_cert_reqs="none") + # 6. tls_ca_file and ssl_cert_reqs combinations + tls_kwargs = _get_tls_kwargs(tls_ca_file="/tmp/ca_certs", ssl_cert_reqs="none") self.assertEqual( tls_kwargs, { @@ -309,7 +309,7 @@ def test_get_tls_kwargs(self): ) tls_kwargs = _get_tls_kwargs( - ssl_ca_certs="/tmp/ca_certs", ssl_cert_reqs="optional" + tls_ca_file="/tmp/ca_certs", ssl_cert_reqs="optional" ) self.assertEqual( tls_kwargs, @@ -322,7 +322,7 @@ def test_get_tls_kwargs(self): ) tls_kwargs = _get_tls_kwargs( - ssl_ca_certs="/tmp/ca_certs", ssl_cert_reqs="required" + tls_ca_file="/tmp/ca_certs", ssl_cert_reqs="required" ) self.assertEqual( tls_kwargs, diff --git a/st2reactor/st2reactor/container/sensor_wrapper.py b/st2reactor/st2reactor/container/sensor_wrapper.py index 8ae1d04740..2330ecd6a8 100644 --- a/st2reactor/st2reactor/container/sensor_wrapper.py +++ b/st2reactor/st2reactor/container/sensor_wrapper.py @@ -235,10 +235,10 @@ def __init__( tls_certificate_key_file=cfg.CONF.database.tls_certificate_key_file, tls_certificate_key_file_password=cfg.CONF.database.tls_certificate_key_file_password, tls_allow_invalid_certificates=cfg.CONF.database.tls_allow_invalid_certificates, + tls_ca_file=cfg.CONF.database.tls_ca_file, ssl_keyfile=cfg.CONF.database.ssl_keyfile, # deprecated / unused ssl_certfile=cfg.CONF.database.ssl_certfile, # deprecated / unused ssl_cert_reqs=cfg.CONF.database.ssl_cert_reqs, # deprecated - ssl_ca_certs=cfg.CONF.database.ssl_ca_certs, authentication_mechanism=cfg.CONF.database.authentication_mechanism, ssl_match_hostname=cfg.CONF.database.ssl_match_hostname, ) From c962d3cded54f66e5f3a2353d0faf4fbf084aa47 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 20 Sep 2024 17:15:01 -0500 Subject: [PATCH 07/15] add database.tls_allow_invalid_hostnames opt to replace ssl_match_hostname For consistency in st2.conf, this uses snake_case not the mongo camelCase option name. --- st2common/st2common/config.py | 15 +++++++++++- st2common/st2common/database_setup.py | 3 ++- st2common/st2common/models/db/__init__.py | 27 +++++++++++++++------- st2common/st2common/persistence/cleanup.py | 6 +++-- st2common/st2common/persistence/db_init.py | 6 +++-- 5 files changed, 43 insertions(+), 14 deletions(-) diff --git a/st2common/st2common/config.py b/st2common/st2common/config.py index 696a83cea5..7c23ba0f38 100644 --- a/st2common/st2common/config.py +++ b/st2common/st2common/config.py @@ -292,9 +292,22 @@ def register_opts(ignore_errors=False): ), ), cfg.BoolOpt( - "ssl_match_hostname", # TODO: replace with "tlsAllowInvalidHostnames" + "tls_allow_invalid_hostnames", + default=None, + sample_default=False, + help=( + "If True and `tlsAllowInvalidCertificates` is True, disables hostname verification. " + "This defaults to False to have security by default. " + "Only temporarily set to True if you need to debug the connection." + ), + ), + cfg.BoolOpt( + "ssl_match_hostname", default=True, help="If True and `ssl_cert_reqs` is not None, enables hostname verification", + deprecated_for_removal=True, + deprecated_reason="Use tls_allow_invalid_hostnames with the opposite value from this option.", + deprecated_since="3.9.0", ), cfg.StrOpt( "authentication_mechanism", diff --git a/st2common/st2common/database_setup.py b/st2common/st2common/database_setup.py index c0a0c2d5f9..a3c67c2411 100644 --- a/st2common/st2common/database_setup.py +++ b/st2common/st2common/database_setup.py @@ -41,11 +41,12 @@ def db_config(): "tls_certificate_key_file_password": cfg.CONF.database.tls_certificate_key_file_password, "tls_allow_invalid_certificates": cfg.CONF.database.tls_allow_invalid_certificates, "tls_ca_file": cfg.CONF.database.tls_ca_file, + "tls_allow_invalid_hostnames": cfg.CONF.database.tls_allow_invalid_hostnames, "ssl_keyfile": cfg.CONF.database.ssl_keyfile, # deprecated / unused "ssl_certfile": cfg.CONF.database.ssl_certfile, # deprecated / unused "ssl_cert_reqs": cfg.CONF.database.ssl_cert_reqs, # deprecated "authentication_mechanism": cfg.CONF.database.authentication_mechanism, - "ssl_match_hostname": cfg.CONF.database.ssl_match_hostname, + "ssl_match_hostname": cfg.CONF.database.ssl_match_hostname, # deprecated } diff --git a/st2common/st2common/models/db/__init__.py b/st2common/st2common/models/db/__init__.py index 873a4ed2f0..11910bec82 100644 --- a/st2common/st2common/models/db/__init__.py +++ b/st2common/st2common/models/db/__init__.py @@ -131,11 +131,12 @@ def _db_connect( tls_certificate_key_file_password=None, tls_allow_invalid_certificates=None, tls_ca_file=None, + tls_allow_invalid_hostnames=None, ssl_keyfile=None, # deprecated / unused ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, # deprecated authentication_mechanism=None, - ssl_match_hostname=True, + ssl_match_hostname=True, # deprecated ): if "://" in db_host: @@ -169,11 +170,12 @@ def _db_connect( tls_certificate_key_file_password=tls_certificate_key_file_password, tls_allow_invalid_certificates=tls_allow_invalid_certificates, tls_ca_file=tls_ca_file, + tls_allow_invalid_hostnames=tls_allow_invalid_hostnames, ssl_keyfile=ssl_keyfile, # deprecated / unused ssl_certfile=ssl_certfile, # deprecated / unused ssl_cert_reqs=ssl_cert_reqs, # deprecated authentication_mechanism=authentication_mechanism, - ssl_match_hostname=ssl_match_hostname, + ssl_match_hostname=ssl_match_hostname, # deprecated ) compressor_kwargs = {} @@ -241,11 +243,12 @@ def db_setup( tls_certificate_key_file_password=None, tls_allow_invalid_certificates=None, tls_ca_file=None, + tls_allow_invalid_hostnames=None, ssl_keyfile=None, # deprecated / unused ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, # deprecated authentication_mechanism=None, - ssl_match_hostname=True, + ssl_match_hostname=True, # deprecated ): connection = _db_connect( @@ -259,11 +262,12 @@ def db_setup( tls_certificate_key_file_password=tls_certificate_key_file_password, tls_allow_invalid_certificates=tls_allow_invalid_certificates, tls_ca_file=tls_ca_file, + tls_allow_invalid_hostnames=tls_allow_invalid_hostnames, ssl_keyfile=ssl_keyfile, # deprecated / unused ssl_certfile=ssl_certfile, # deprecated / unused ssl_cert_reqs=ssl_cert_reqs, # deprecated authentication_mechanism=authentication_mechanism, - ssl_match_hostname=ssl_match_hostname, + ssl_match_hostname=ssl_match_hostname, # deprecated ) # Create all the indexes upfront to prevent race-conditions caused by @@ -413,11 +417,12 @@ def db_cleanup( tls_certificate_key_file_password=None, tls_allow_invalid_certificates=None, tls_ca_file=None, + tls_allow_invalid_hostnames=None, ssl_keyfile=None, # deprecated / unused ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, # deprecated authentication_mechanism=None, - ssl_match_hostname=True, + ssl_match_hostname=True, # deprecated ): connection = _db_connect( @@ -431,11 +436,12 @@ def db_cleanup( tls_certificate_key_file_password=tls_certificate_key_file_password, tls_allow_invalid_certificates=tls_allow_invalid_certificates, tls_ca_file=tls_ca_file, + tls_allow_invalid_hostnames=tls_allow_invalid_hostnames, ssl_keyfile=ssl_keyfile, # deprecated / unused ssl_certfile=ssl_certfile, # deprecated / unused ssl_cert_reqs=ssl_cert_reqs, # deprecated authentication_mechanism=authentication_mechanism, - ssl_match_hostname=ssl_match_hostname, + ssl_match_hostname=ssl_match_hostname, # deprecated ) LOG.info( @@ -456,11 +462,12 @@ def _get_tls_kwargs( tls_certificate_key_file_password=None, tls_allow_invalid_certificates=None, tls_ca_file=None, + tls_allow_invalid_hostnames=None, ssl_keyfile=None, # deprecated / unused ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, # deprecated authentication_mechanism=None, - ssl_match_hostname=True, + ssl_match_hostname=True, # deprecated ): # NOTE: In pymongo 3.9.0 some of the ssl related arguments have been renamed - # https://api.mongodb.com/python/current/changelog.html#changes-in-version-3-9-0 @@ -493,7 +500,11 @@ def _get_tls_kwargs( if tls_kwargs.get("tls", False): # pass in tlsAllowInvalidHostname only if tls is True. The right default value # for tlsAllowInvalidHostname in almost all cases is False. - tls_kwargs["tlsAllowInvalidHostnames"] = not ssl_match_hostname + tls_kwargs["tlsAllowInvalidHostnames"] = ( + tls_allow_invalid_hostnames + if tls_allow_invalid_hostnames is not None + else not ssl_match_hostname + ) return tls_kwargs diff --git a/st2common/st2common/persistence/cleanup.py b/st2common/st2common/persistence/cleanup.py index d807c63b98..c0dc3af850 100644 --- a/st2common/st2common/persistence/cleanup.py +++ b/st2common/st2common/persistence/cleanup.py @@ -49,11 +49,12 @@ def db_cleanup_with_retry( tls_certificate_key_file_password=None, tls_allow_invalid_certificates=None, tls_ca_file=None, + tls_allow_invalid_hostnames=None, ssl_keyfile=None, # deprecated / unused ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, # deprecated authentication_mechanism=None, - ssl_match_hostname=True, + ssl_match_hostname=True, # deprecated ): """ This method is a retry version of db_cleanup. @@ -70,11 +71,12 @@ def db_cleanup_with_retry( tls_certificate_key_file_password=tls_certificate_key_file_password, tls_allow_invalid_certificates=tls_allow_invalid_certificates, tls_ca_file=tls_ca_file, + tls_allow_invalid_hostnames=tls_allow_invalid_hostnames, ssl_keyfile=ssl_keyfile, # deprecated / unused ssl_certfile=ssl_certfile, # deprecated / unused ssl_cert_reqs=ssl_cert_reqs, # deprecated authentication_mechanism=authentication_mechanism, - ssl_match_hostname=ssl_match_hostname, + ssl_match_hostname=ssl_match_hostname, # deprecated ) diff --git a/st2common/st2common/persistence/db_init.py b/st2common/st2common/persistence/db_init.py index 62a14826f5..0e091826ac 100644 --- a/st2common/st2common/persistence/db_init.py +++ b/st2common/st2common/persistence/db_init.py @@ -70,11 +70,12 @@ def db_setup_with_retry( tls_certificate_key_file_password=None, tls_allow_invalid_certificates=None, tls_ca_file=None, + tls_allow_invalid_hostnames=None, ssl_keyfile=None, # deprecated / unused ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, # deprecated authentication_mechanism=None, - ssl_match_hostname=True, + ssl_match_hostname=True, # deprecated ): """ This method is a retry version of db_setup. @@ -92,9 +93,10 @@ def db_setup_with_retry( tls_certificate_key_file_password=tls_certificate_key_file_password, tls_allow_invalid_certificates=tls_allow_invalid_certificates, tls_ca_file=tls_ca_file, + tls_allow_invalid_hostnames=tls_allow_invalid_hostnames, ssl_keyfile=ssl_keyfile, # deprecated / unused ssl_certfile=ssl_certfile, # deprecated / unused ssl_cert_reqs=ssl_cert_reqs, # deprecated authentication_mechanism=authentication_mechanism, - ssl_match_hostname=ssl_match_hostname, + ssl_match_hostname=ssl_match_hostname, # deprecated ) From 6b044d7dfcc08522f833f72ec6979bff2154225c Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 20 Sep 2024 17:27:05 -0500 Subject: [PATCH 08/15] Update comment saying database options have been migrated to new names --- st2common/st2common/models/db/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/st2common/models/db/__init__.py b/st2common/st2common/models/db/__init__.py index 11910bec82..7edc84efd1 100644 --- a/st2common/st2common/models/db/__init__.py +++ b/st2common/st2common/models/db/__init__.py @@ -471,7 +471,7 @@ def _get_tls_kwargs( ): # NOTE: In pymongo 3.9.0 some of the ssl related arguments have been renamed - # https://api.mongodb.com/python/current/changelog.html#changes-in-version-3-9-0 - # Old names stop working in pymongo 4, so we need to migrate now: + # Old names stopped working in pymongo 4, so we migrated to the new names in st2 3.9.0. # https://pymongo.readthedocs.io/en/stable/migrate-to-pymongo4.html#renamed-uri-options tls_kwargs = { "tls": tls, From 6800a7aee1f65e91459479cdc2a5456aa63559a3 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 20 Sep 2024 17:28:35 -0500 Subject: [PATCH 09/15] Add oslo's secret=True on database.password opt Not sure if this wasn't available before, or why it wasn't used. Try and see. --- st2common/st2common/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/st2common/config.py b/st2common/st2common/config.py index 7c23ba0f38..c58009453f 100644 --- a/st2common/st2common/config.py +++ b/st2common/st2common/config.py @@ -184,7 +184,7 @@ def register_opts(ignore_errors=False): cfg.IntOpt("port", default=27017, help="port of db server"), cfg.StrOpt("db_name", default="st2", help="name of database"), cfg.StrOpt("username", help="username for db login"), - cfg.StrOpt("password", help="password for db login"), + cfg.StrOpt("password", help="password for db login", secret=True), cfg.IntOpt( "connection_timeout", default=3 * 1000, From 3a5a1e62401e4b5c2797ccb07a70dd9097427267 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 20 Sep 2024 17:35:47 -0500 Subject: [PATCH 10/15] database opts: do not pass around unused deprecated opts --- contrib/packs/actions/pack_mgmt/unload.py | 2 -- st2common/st2common/database_setup.py | 2 -- st2common/st2common/models/db/__init__.py | 14 -------------- st2common/st2common/persistence/cleanup.py | 4 ---- st2common/st2common/persistence/db_init.py | 4 ---- st2common/tests/unit/test_db.py | 12 +----------- st2reactor/st2reactor/container/sensor_wrapper.py | 2 -- 7 files changed, 1 insertion(+), 39 deletions(-) diff --git a/contrib/packs/actions/pack_mgmt/unload.py b/contrib/packs/actions/pack_mgmt/unload.py index c70663e4dc..2d4aaf989d 100644 --- a/contrib/packs/actions/pack_mgmt/unload.py +++ b/contrib/packs/actions/pack_mgmt/unload.py @@ -64,8 +64,6 @@ def initialize(self): tls_certificate_key_file_password=cfg.CONF.database.tls_certificate_key_file_password, tls_allow_invalid_certificates=cfg.CONF.database.tls_allow_invalid_certificates, tls_ca_file=cfg.CONF.database.tls_ca_file, - ssl_keyfile=cfg.CONF.database.ssl_keyfile, # deprecated / unused - ssl_certfile=cfg.CONF.database.ssl_certfile, # deprecated / unused ssl_cert_reqs=cfg.CONF.database.ssl_cert_reqs, # deprecated authentication_mechanism=cfg.CONF.database.authentication_mechanism, ssl_match_hostname=cfg.CONF.database.ssl_match_hostname, diff --git a/st2common/st2common/database_setup.py b/st2common/st2common/database_setup.py index a3c67c2411..68b7583942 100644 --- a/st2common/st2common/database_setup.py +++ b/st2common/st2common/database_setup.py @@ -42,8 +42,6 @@ def db_config(): "tls_allow_invalid_certificates": cfg.CONF.database.tls_allow_invalid_certificates, "tls_ca_file": cfg.CONF.database.tls_ca_file, "tls_allow_invalid_hostnames": cfg.CONF.database.tls_allow_invalid_hostnames, - "ssl_keyfile": cfg.CONF.database.ssl_keyfile, # deprecated / unused - "ssl_certfile": cfg.CONF.database.ssl_certfile, # deprecated / unused "ssl_cert_reqs": cfg.CONF.database.ssl_cert_reqs, # deprecated "authentication_mechanism": cfg.CONF.database.authentication_mechanism, "ssl_match_hostname": cfg.CONF.database.ssl_match_hostname, # deprecated diff --git a/st2common/st2common/models/db/__init__.py b/st2common/st2common/models/db/__init__.py index 7edc84efd1..018a419c17 100644 --- a/st2common/st2common/models/db/__init__.py +++ b/st2common/st2common/models/db/__init__.py @@ -132,8 +132,6 @@ def _db_connect( tls_allow_invalid_certificates=None, tls_ca_file=None, tls_allow_invalid_hostnames=None, - ssl_keyfile=None, # deprecated / unused - ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, # deprecated authentication_mechanism=None, ssl_match_hostname=True, # deprecated @@ -171,8 +169,6 @@ def _db_connect( tls_allow_invalid_certificates=tls_allow_invalid_certificates, tls_ca_file=tls_ca_file, tls_allow_invalid_hostnames=tls_allow_invalid_hostnames, - ssl_keyfile=ssl_keyfile, # deprecated / unused - ssl_certfile=ssl_certfile, # deprecated / unused ssl_cert_reqs=ssl_cert_reqs, # deprecated authentication_mechanism=authentication_mechanism, ssl_match_hostname=ssl_match_hostname, # deprecated @@ -244,8 +240,6 @@ def db_setup( tls_allow_invalid_certificates=None, tls_ca_file=None, tls_allow_invalid_hostnames=None, - ssl_keyfile=None, # deprecated / unused - ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, # deprecated authentication_mechanism=None, ssl_match_hostname=True, # deprecated @@ -263,8 +257,6 @@ def db_setup( tls_allow_invalid_certificates=tls_allow_invalid_certificates, tls_ca_file=tls_ca_file, tls_allow_invalid_hostnames=tls_allow_invalid_hostnames, - ssl_keyfile=ssl_keyfile, # deprecated / unused - ssl_certfile=ssl_certfile, # deprecated / unused ssl_cert_reqs=ssl_cert_reqs, # deprecated authentication_mechanism=authentication_mechanism, ssl_match_hostname=ssl_match_hostname, # deprecated @@ -418,8 +410,6 @@ def db_cleanup( tls_allow_invalid_certificates=None, tls_ca_file=None, tls_allow_invalid_hostnames=None, - ssl_keyfile=None, # deprecated / unused - ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, # deprecated authentication_mechanism=None, ssl_match_hostname=True, # deprecated @@ -437,8 +427,6 @@ def db_cleanup( tls_allow_invalid_certificates=tls_allow_invalid_certificates, tls_ca_file=tls_ca_file, tls_allow_invalid_hostnames=tls_allow_invalid_hostnames, - ssl_keyfile=ssl_keyfile, # deprecated / unused - ssl_certfile=ssl_certfile, # deprecated / unused ssl_cert_reqs=ssl_cert_reqs, # deprecated authentication_mechanism=authentication_mechanism, ssl_match_hostname=ssl_match_hostname, # deprecated @@ -463,8 +451,6 @@ def _get_tls_kwargs( tls_allow_invalid_certificates=None, tls_ca_file=None, tls_allow_invalid_hostnames=None, - ssl_keyfile=None, # deprecated / unused - ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, # deprecated authentication_mechanism=None, ssl_match_hostname=True, # deprecated diff --git a/st2common/st2common/persistence/cleanup.py b/st2common/st2common/persistence/cleanup.py index c0dc3af850..f633e576c7 100644 --- a/st2common/st2common/persistence/cleanup.py +++ b/st2common/st2common/persistence/cleanup.py @@ -50,8 +50,6 @@ def db_cleanup_with_retry( tls_allow_invalid_certificates=None, tls_ca_file=None, tls_allow_invalid_hostnames=None, - ssl_keyfile=None, # deprecated / unused - ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, # deprecated authentication_mechanism=None, ssl_match_hostname=True, # deprecated @@ -72,8 +70,6 @@ def db_cleanup_with_retry( tls_allow_invalid_certificates=tls_allow_invalid_certificates, tls_ca_file=tls_ca_file, tls_allow_invalid_hostnames=tls_allow_invalid_hostnames, - ssl_keyfile=ssl_keyfile, # deprecated / unused - ssl_certfile=ssl_certfile, # deprecated / unused ssl_cert_reqs=ssl_cert_reqs, # deprecated authentication_mechanism=authentication_mechanism, ssl_match_hostname=ssl_match_hostname, # deprecated diff --git a/st2common/st2common/persistence/db_init.py b/st2common/st2common/persistence/db_init.py index 0e091826ac..a8fa2711fe 100644 --- a/st2common/st2common/persistence/db_init.py +++ b/st2common/st2common/persistence/db_init.py @@ -71,8 +71,6 @@ def db_setup_with_retry( tls_allow_invalid_certificates=None, tls_ca_file=None, tls_allow_invalid_hostnames=None, - ssl_keyfile=None, # deprecated / unused - ssl_certfile=None, # deprecated / unused ssl_cert_reqs=None, # deprecated authentication_mechanism=None, ssl_match_hostname=True, # deprecated @@ -94,8 +92,6 @@ def db_setup_with_retry( tls_allow_invalid_certificates=tls_allow_invalid_certificates, tls_ca_file=tls_ca_file, tls_allow_invalid_hostnames=tls_allow_invalid_hostnames, - ssl_keyfile=ssl_keyfile, # deprecated / unused - ssl_certfile=ssl_certfile, # deprecated / unused ssl_cert_reqs=ssl_cert_reqs, # deprecated authentication_mechanism=authentication_mechanism, ssl_match_hostname=ssl_match_hostname, # deprecated diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 03742aa003..89814fdf87 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -234,7 +234,7 @@ def test_get_tls_kwargs(self): tls_kwargs = _get_tls_kwargs(tls=True) self.assertEqual(tls_kwargs, {"tls": True, "tlsAllowInvalidHostnames": False}) - # 2. authentication_mechanism kwarg provided + # 3. authentication_mechanism kwarg provided tls_kwargs = _get_tls_kwargs(authentication_mechanism="MONGODB-X509") self.assertEqual( tls_kwargs, @@ -245,16 +245,6 @@ def test_get_tls_kwargs(self): }, ) - # 3. ssl_keyfile and ssl_certfile are ignored by pymongo so this does too. - tls_kwargs = _get_tls_kwargs(ssl_keyfile="/tmp/keyfile", ssl_certfile="/tmp/certfile") - self.assertEqual( - tls_kwargs, - { - "tls": True, - "tlsAllowInvalidHostnames": False, - }, - ) - # 4a. tls_certificate_key_file provided tls_kwargs = _get_tls_kwargs(tls_certificate_key_file="/tmp/keyfile") self.assertEqual( diff --git a/st2reactor/st2reactor/container/sensor_wrapper.py b/st2reactor/st2reactor/container/sensor_wrapper.py index 2330ecd6a8..cc4723043a 100644 --- a/st2reactor/st2reactor/container/sensor_wrapper.py +++ b/st2reactor/st2reactor/container/sensor_wrapper.py @@ -236,8 +236,6 @@ def __init__( tls_certificate_key_file_password=cfg.CONF.database.tls_certificate_key_file_password, tls_allow_invalid_certificates=cfg.CONF.database.tls_allow_invalid_certificates, tls_ca_file=cfg.CONF.database.tls_ca_file, - ssl_keyfile=cfg.CONF.database.ssl_keyfile, # deprecated / unused - ssl_certfile=cfg.CONF.database.ssl_certfile, # deprecated / unused ssl_cert_reqs=cfg.CONF.database.ssl_cert_reqs, # deprecated authentication_mechanism=cfg.CONF.database.authentication_mechanism, ssl_match_hostname=cfg.CONF.database.ssl_match_hostname, From 8e185f5541a7b50cf300f9a38511fd76517ddda5 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Mon, 23 Sep 2024 21:38:55 -0500 Subject: [PATCH 11/15] pants-plugins/sample_conf: fail fmt if the config_gen process fails --- pants-plugins/sample_conf/rules.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pants-plugins/sample_conf/rules.py b/pants-plugins/sample_conf/rules.py index 97195943d5..675b7cf6d8 100644 --- a/pants-plugins/sample_conf/rules.py +++ b/pants-plugins/sample_conf/rules.py @@ -27,7 +27,7 @@ FileContent, Snapshot, ) -from pants.engine.process import FallibleProcessResult +from pants.engine.process import ProcessResult from pants.engine.rules import Get, collect_rules, rule from pants.engine.target import FieldSet from pants.util.logging import LogLevel @@ -64,7 +64,7 @@ async def generate_sample_conf_via_fmt( pex = await Get(VenvPex, PexFromTargetsRequest, subsystem.pex_request()) result = await Get( - FallibleProcessResult, + ProcessResult, VenvPexProcess( pex, description="Regenerating st2.conf.sample", From bdc0bcf99100d51ea1c294c7d44a38b629f5f05e Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Mon, 23 Sep 2024 21:43:22 -0500 Subject: [PATCH 12/15] tools/config_gen: Use sample_default when available And use fix the sample default for python_binary to use python3. --- conf/st2.conf.sample | 2 +- st2common/st2common/config.py | 3 +++ tools/config_gen.py | 18 +++++++++++------- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/conf/st2.conf.sample b/conf/st2.conf.sample index 82da2ae2e1..2652e3ae26 100644 --- a/conf/st2.conf.sample +++ b/conf/st2.conf.sample @@ -19,7 +19,7 @@ logging = /etc/st2/logging.actionrunner.conf # List of pip options to be passed to "pip install" command when installing pack dependencies into pack virtual environment. pip_opts = # comma separated list allowed here. # Python binary which will be used by Python actions. -python_binary = /usr/bin/python +python_binary = /usr/bin/python3 # Default log level to use for Python runner actions. Can be overriden on invocation basis using "log_level" runner parameter. python_runner_log_level = DEBUG # Time interval between subsequent queries to check running executions. diff --git a/st2common/st2common/config.py b/st2common/st2common/config.py index c58009453f..53a48ff174 100644 --- a/st2common/st2common/config.py +++ b/st2common/st2common/config.py @@ -172,6 +172,7 @@ def register_opts(ignore_errors=False): cfg.StrOpt( "webui_base_url", default="https://%s" % socket.getfqdn(), + sample_default="https://localhost", help="Base https URL to access st2 Web UI. This is used to construct history URLs " "that are sent out when chatops is used to kick off executions.", ) @@ -533,11 +534,13 @@ def register_opts(ignore_errors=False): cfg.StrOpt( "python_binary", default=default_python_bin_path, + sample_default="/usr/bin/python3", help="Python binary which will be used by Python actions.", ), cfg.StrOpt( "virtualenv_binary", default=default_virtualenv_bin_path, + sample_default="/usr/bin/virtualenv", help="Virtualenv binary which should be used to create pack virtualenvs.", ), cfg.StrOpt( diff --git a/tools/config_gen.py b/tools/config_gen.py index f139474c13..0881526aaa 100755 --- a/tools/config_gen.py +++ b/tools/config_gen.py @@ -76,7 +76,7 @@ STATIC_OPTION_VALUES = { "actionrunner": { "virtualenv_binary": "/usr/bin/virtualenv", - "python_binary": "/usr/bin/python", + "python_binary": "/usr/bin/python3", }, "webui": {"webui_base_url": "https://localhost"}, } @@ -164,27 +164,31 @@ def _print_options(opt_group, options): if opt.name in SKIP_OPTIONS: continue + opt_default = opt.default if opt.sample_default is None else opt.sample_default + # Special case for options which could change during this script run static_option_value = STATIC_OPTION_VALUES.get(opt_group.name, {}).get( opt.name, None ) if static_option_value: - opt.default = static_option_value + assert ( + opt_default == static_option_value + ), f"opt_default={opt_default} != static_option_value={static_option_value}" # Special handling for list options if isinstance(opt, cfg.ListOpt): - if opt.default: - value = ",".join(opt.default) + if opt_default: + value = ",".join(opt_default) else: value = "" value += " # comma separated list allowed here." - elif isinstance(opt.default, dict): + elif isinstance(opt_default, dict): # this is for [sensorcontainer].partition_provider which # is a generic cfg.Opt(type=types.Dict(value_type=types.String()) - value = " ".join([f"{k}:{v}" for k, v in opt.default.items()]) + value = " ".join([f"{k}:{v}" for k, v in opt_default.items()]) else: - value = opt.default + value = opt_default print(("# %s" % opt.help).strip()) From d18705b092b8e636dd9ba6ab6012505d1aa9ed91 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Mon, 23 Sep 2024 21:44:18 -0500 Subject: [PATCH 13/15] tools/config_gen: Report on deprecated options in st2.conf.sample --- conf/st2.conf.sample | 6 ++++-- st2common/st2common/config.py | 12 ++++++++---- tools/config_gen.py | 14 ++++++++++++++ 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/conf/st2.conf.sample b/conf/st2.conf.sample index 2652e3ae26..93314d7975 100644 --- a/conf/st2.conf.sample +++ b/conf/st2.conf.sample @@ -106,11 +106,13 @@ index_url = https://index.stackstorm.org/v1/index.json # comma separated list al pack_group = st2packs # Paths which will be searched for integration packs. packs_base_paths = None -# Paths which will be searched for runners. NOTE: This option has been deprecated and it's unused since StackStorm v3.0.0 +# Paths which will be searched for runners. +# DEPRECATED FOR REMOVAL since 3.0.0: Option unused since StackStorm v3.0.0 runners_base_paths = None # Path to the directory which contains system packs. system_packs_base_path = /opt/stackstorm/packs -# Path to the directory which contains system runners. NOTE: This option has been deprecated and it's unused since StackStorm v3.0.0 +# Path to the directory which contains system runners. +# DEPRECATED FOR REMOVAL since 3.0.0: Option unused since StackStorm v3.0.0 system_runners_base_path = /opt/stackstorm/runners [coordination] diff --git a/st2common/st2common/config.py b/st2common/st2common/config.py index 53a48ff174..bbf73752b3 100644 --- a/st2common/st2common/config.py +++ b/st2common/st2common/config.py @@ -143,8 +143,10 @@ def register_opts(ignore_errors=False): cfg.StrOpt( "system_runners_base_path", default=system_runners_base_path, - help="Path to the directory which contains system runners. " - "NOTE: This option has been deprecated and it's unused since StackStorm v3.0.0", + help="Path to the directory which contains system runners.", + deprecated_for_removal=True, + deprecated_reason="Option unused since StackStorm v3.0.0", + deprecated_since="3.0.0", ), cfg.StrOpt( "packs_base_paths", @@ -154,8 +156,10 @@ def register_opts(ignore_errors=False): cfg.StrOpt( "runners_base_paths", default=None, - help="Paths which will be searched for runners. " - "NOTE: This option has been deprecated and it's unused since StackStorm v3.0.0", + help="Paths which will be searched for runners.", + deprecated_for_removal=True, + deprecated_reason="Option unused since StackStorm v3.0.0", + deprecated_since="3.0.0", ), cfg.ListOpt( "index_url", diff --git a/tools/config_gen.py b/tools/config_gen.py index 0881526aaa..972171af71 100755 --- a/tools/config_gen.py +++ b/tools/config_gen.py @@ -192,6 +192,20 @@ def _print_options(opt_group, options): print(("# %s" % opt.help).strip()) + for deprecated_opt in opt.deprecated_opts: + deprecated_opt: cfg.DeprecatedOpt + alias = ( + deprecated_opt.name + if deprecated_opt.group is None + else f"{deprecated_opt.group}.{deprecated_opt.name}" + ) + print(f"# This option has a deprecated alias: {alias}") + + if opt.deprecated_for_removal: + print( + f"# DEPRECATED FOR REMOVAL since {opt.deprecated_since}: {opt.deprecated_reason}".strip() + ) + if isinstance(opt, cfg.StrOpt) and opt.type.choices: if isinstance(opt.type.choices, OrderedDict): valid_values = ", ".join([str(x) for x in opt.type.choices]) From 9a045abc3ab9ed140f4f227f7e13a5ec4462d738 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Mon, 23 Sep 2024 21:44:46 -0500 Subject: [PATCH 14/15] Regenerate st2.conf.sample to include new tls opts --- conf/st2.conf.sample | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/conf/st2.conf.sample b/conf/st2.conf.sample index 93314d7975..5eed1ffd4c 100644 --- a/conf/st2.conf.sample +++ b/conf/st2.conf.sample @@ -144,19 +144,33 @@ host = 127.0.0.1 password = None # port of db server port = 27017 -# Create the connection to mongodb using SSL -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 +# DEPRECATED FOR REMOVAL since 3.9.0: Use tls_allow_invalid_certificates with the following: The 'optional' and 'required' values are equivalent to tls_allow_invalid_certificates=False. The 'none' value is equivalent to tls_allow_invalid_certificates=True. This option is a needlessly more complex version of tls_allow_invalid_certificates. # Valid values: none, optional, required ssl_cert_reqs = None # Certificate file used to identify the localconnection +# DEPRECATED FOR REMOVAL since 3.9.0: Use tls_certificate_key_file with a path to a file containing the concatenation of the files from ssl_keyfile and ssl_certfile. This option is ignored by pymongo. ssl_certfile = None # Private keyfile used to identify the local connection against MongoDB. +# DEPRECATED FOR REMOVAL since 3.9.0: Use tls_certificate_key_file with a path to a file containing the concatenation of the files from ssl_keyfile and ssl_certfile. This option is ignored by pymongo. ssl_keyfile = None # If True and `ssl_cert_reqs` is not None, enables hostname verification +# DEPRECATED FOR REMOVAL since 3.9.0: Use tls_allow_invalid_hostnames with the opposite value from this option. ssl_match_hostname = True +# Create the connection to mongodb using TLS. +# This option has a deprecated alias: ssl +tls = False +# Specifies whether MongoDB is allowed to pass an invalid certificate. This defaults to False to have security by default. Only temporarily set to True if you need to debug the connection. +tls_allow_invalid_certificates = False +# If True and `tlsAllowInvalidCertificates` is True, disables hostname verification. This defaults to False to have security by default. Only temporarily set to True if you need to debug the connection. +tls_allow_invalid_hostnames = False +# ca_certs file contains a set of concatenated CA certificates, which are used to validate certificates passed from MongoDB. +# This option has a deprecated alias: ssl_ca_certs +tls_ca_file = None +# Client certificate used to identify the local connection against MongoDB. The certificate file must contain one or both of private key and certificate. Supplying separate files for private key (ssl_keyfile) and certificate (ssl_certfile) is no longer supported. If encrypted, pass the password or passphrase in tls_certificate_key_file_password. +tls_certificate_key_file = None +# The password or passphrase to decrypt the file in tls_certificate_key_file. Only set this if tls_certificate_key_file is encrypted. +tls_certificate_key_file_password = None # username for db login username = None # Compression level when compressors is set to zlib. Valid values are -1 to 9. Defaults to 6. From 059bca37b2393da03607c2b9e49637355882cdc5 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Mon, 23 Sep 2024 22:09:24 -0500 Subject: [PATCH 15/15] add upgrade notes to CHANGELOG --- CHANGELOG.rst | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index e6a4991b6e..29ebb59719 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -6,6 +6,21 @@ in development Python 3.6 is no longer supported; Stackstorm requires at least Python 3.8. +Several st2.conf database options have been renamed or deprecated. Most of the options will continue to work using their old name. +However, if you use `[database].ssl_keyfile` and/or `[database].ssl_certfile`, you MUST migrate to `[database].tls_certificate_key_file`. +This new option expects the key and certificate in the same file. Use something like the following to create that file from your old files: + +``` +cat path/to/ssl_keyfile path/to/ssl_certfile > path/to/tls_certificate_key_file +``` + +Other options that were renamed under `[database]` are (more details available in `st2.conf.sample`): + +* `ssl` -> `tls` +* `ssl_cert_reqs` -> `tls_allow_invalid_certificates` (opt type change: string -> boolean) +* `ssl_ca_certs` -> `tls_ca_file` +* `ssl_match_hostnames` -> `tls_allow_invalid_hostnames` (meaning is inverted: the new option is the opposite of the old) + Fixed ~~~~~ * Fixed #6021 and #5327 by adding max_page_size to api_opts and added limit and offset to list_values() methods of @@ -31,6 +46,11 @@ Changed * Updated unit tests to use redis for coordination instead of the NoOp driver. This will hopefully make CI more stable. #6245 Contributed by @FileMagic, @guzzijones, and @cognifloyd +* Renamed `[database].ssl*` options to support pymongo 4, which we have to update to support newer MongoDB servers. + Please see the note above about migrating to the newer options, especially if you use `[database].ssl_keyfile` + and/or `[database].ssl_certfile`, as those options are ignored in StackStorm 3.9.0. #6250 + Contributed by @cognifloyd + Added ~~~~~ * Continue introducing `pants `_ to improve DX (Developer Experience)