Fix: support TLS for HTTP and MQ connections#7
Merged
Conversation
9fa55f1 to
38fd70e
Compare
Previously the client hardcoded plaintext http:// for the aiohttp base_url and used plaintext AMQP for aio_pika.connect_robust(...). When the P2P daemons are not co-located with the client, credentials (including the MQ password) traverse the network in cleartext. This change adds four optional parameters to make_p2p_service_client: - http_use_tls / http_ssl_context: when enabled, the base URL uses https:// and aiohttp gets a TCPConnector with the provided SSLContext (or True for the default verifying context). - mq_use_tls / mq_ssl_options: plumbed through declare_mq_objects to aio_pika.connect_robust as ssl=... and ssl_options=... (aio_pika's native dict-based API). Defaults are unchanged: with both *_use_tls=False, existing localhost callers behave exactly as before.
38fd70e to
ab48446
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes security finding M1 from the internal review: the client previously hardcoded plaintext
http://for the aiohttp base URL and used plaintext AMQP foraio_pika.connect_robust(...). When the P2P daemons are not co-located with the client, credentials (including the MQ password) traverse the network in cleartext.This PR adds opt-in TLS support to
make_p2p_service_clientso cross-host deployments can encrypt both transports.New parameters
make_p2p_service_clientgains four optional parameters:http_use_tls: bool = False- whenTrue, the base URL useshttps://instead ofhttp://.http_ssl_context: Optional[ssl.SSLContext] = None- optional customSSLContext. Whenhttp_use_tls=Trueand this isNone, aiohttp'sTCPConnectorreceivesssl=True(default verifying context). When provided, it is passed through verbatim.mq_use_tls: bool = False- plumbed toaio_pika.connect_robust(..., ssl=...).mq_ssl_options: Optional[Dict[str, Any]] = None- plumbed toaio_pika.connect_robust(..., ssl_options=...). This is aio_pika's native dict-based API; no translation layer is added.The MQ parameters are also threaded through
declare_mq_objectsso it remains usable directly.Behavior change
None for existing callers. With both
http_use_tls=Falseandmq_use_tls=False(the defaults), the factory takes the same code path as before: plaintexthttp://base URL andaio_pika.connect_robustcalled withssl=False, ssl_options=None. Existing localhost callers are unaffected.Out of scope
Credential defaults (e.g. removing
guest:guest) and HTTP request timeouts are intentionally left for separate PRs.Test plan
http_use_tls=Trueagainst an HTTPS-enabled P2P daemon successfully completesidentify().mq_use_tls=True(with appropriatemq_ssl_optionsand an AMQPS-enabled broker) successfully publishes and subscribes.http_ssl_context(e.g. with a private CA) is honored by the connector.