-
Notifications
You must be signed in to change notification settings - Fork 256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for passing in connection arg or existing SSL connection to aiomysql.connect() #757
Comments
Hi, I believe what you're asking is already possible. You can pass an |
just leaving this here to link the related issue on cloud-sql-python-connector's side. |
I think I misunderstood your request at first, as you'd not only need the custom API wise, having Do I understand it correctly, that the cloud connector logic would basically replace the current STARTTLS-like implementation with explicit TLS? |
@Nothing4You Thanks for the response! Yes it seems like you understand the issue correctly. The Cloud SQL Connector takes care of the TLS configuration independent of the database protocol. Which means we require a way to skip the database level SSL/TLS exchange. I would agree that Happy to hop on a call and discuss this further if more details are required or to see how we can collaborate on a potential solution. Thanks so much and have a great day! |
I'm trying to think of a good way to include a test case for this that would include explicit TLS, do you have an idea for this? |
Here is where we configure our A test case could be to stub/mock out a method around where the SSL/TLS exchange is occurring at the database level and assert that it isn't called when using explicit TLS. That is just a quick thought, but again I'm happy to setup some time to chat on a call and discuss implementation/testing approaches :) Let me know, thanks for the help and have a great day! |
@Nothing4You do you have any guidance on which portion of the code would need to be updated in order to implement this feat? I'm happy to attempt to implement the required changes myself if you are able to provide a little guidance into where and how you think the implementation should go? Thanks so much in advance, let me know :) |
Hi Jack, I'm sorry, I've been quite busy in the last days. In our current test cases we don't have mock (as in fake) DB connections, all tests run against real servers. The implementation for this (without the test) is quite small, I hope I'll have some time by the weekend to include an option for this in a separate branch as a starting point. |
No worries at all. I appreciated the help, I am looking forward to playing around with the new branch once it is ready and I can try and help with tests once I see the code. Thanks so much for looking into this! |
…onnections, such as Google Cloud SQL fixes aio-libs#757
Just realized that my terminology was off apparently. I've just created #786 as an option for how to implement this. |
Hi @Nothing4You thanks for putting up that branch, much appreciated. I've taken a little time to play around with it. It seems there is one main spot in the code that is giving me some troubles. Is it this line within
Let me know if you have any thoughts or ideas as to what may be the source of this. I will also continue to play around with it. Thanks again for all the help! |
File "/usr/local/google/home/jackwoth/.pyenv/versions/3.10.1/lib/python3.10/site-packages/aiomysql/connection.py", line 543, in _connect
await self._get_server_information() This is the key, it's still doing some other communication before it hits the logic in Looks like it's a little more complicated to patch this in. I'll be mostly unavailable for the rest of the week probably, but I should be able to update the PR early next week, unless you want to provide a patch for the PR before that? |
I think I'm getting pretty close, I skipped
There is mostly likely a step I am missing that I need to add into Thanks again for all the help! |
I just pushed an update to the PR, can you try again with those changes? |
Looks like I'm still getting a similar error but I will play around a bit and see if I can get it to work.
|
…onnections, such as Google Cloud SQL fixes aio-libs#757
…onnections, such as Google Cloud SQL fixes aio-libs#757
Just pushed an update to #786, that seems to work in my local test with a TLS-terminating HAproxy in the middle. |
@Nothing4You Seems to work for the Cloud SQL Python Connector as well! Thanks for the help on this. Any next steps needed in order to get this into a release? |
next steps are primarily updating the test suite for this. |
Awesome, just took a look. Is there anything I can help on for the checklist? I'm not too familiar with HAproxy unfortunately. |
@Nothing4You I have a draft PR for supporting |
Hey @jackwotherspoon, I've tested with the following haproxy config (needs global
log stdout format raw local0
ssl-default-bind-ciphers ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384
ssl-default-bind-ciphersuites TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256
ssl-default-bind-options prefer-client-ciphers no-sslv3 no-tlsv10 no-tlsv11 no-tls-tickets
ssl-default-server-ciphers ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-SHA384
ssl-default-server-ciphersuites TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256
ssl-default-server-options no-sslv3 no-tlsv10 no-tlsv11 no-tls-tickets
defaults
log global
mode http
option httplog
option dontlognull
timeout connect 10s
timeout client 1m
timeout server 30m
timeout http-keep-alive 10s
option tcpka
option redispatch
option allbackups
frontend tcp-13306-front
bind 127.0.0.1:13306 ssl crt haproxy.pem
bind [::1]:13306 ssl crt haproxy.pem
mode tcp
option tcplog
option tcp-smart-accept
option logasap
default_backend tcp-13306-backend
backend tcp-13306-backend
mode tcp
option tcp-check
option tcp-smart-connect
server localhost 127.0.0.1:3306 it could probably be simplified a bit and the upstream server can probably use docker dns (maybe something like https://stackoverflow.com/a/61655830). if you want, you could look into creating the tests i listed in #786, but I'm not sure it's worth the time investment on your end right now, it shouldn't take too much of my time, i just can't say yet when i'll have time to do this. |
@Nothing4You I will definitely give the entire change a review when you are ready! Thanks again so much for the work on this, super appreciated! :) Excited to allow our users to connect to Cloud SQL via |
…onnections, such as Google Cloud SQL fixes aio-libs#757
…onnections, such as Google Cloud SQL fixes aio-libs#757
…onnections, such as Google Cloud SQL fixes aio-libs#757
…onnections, such as Google Cloud SQL fixes aio-libs#757
…onnections, such as Google Cloud SQL fixes aio-libs#757
…onnections, such as Google Cloud SQL fixes aio-libs#757
…onnections, such as Google Cloud SQL fixes aio-libs#757
…onnections, such as Google Cloud SQL fixes aio-libs#757
…onnections, such as Google Cloud SQL fixes aio-libs#757
…onnections, such as Google Cloud SQL fixes aio-libs#757
@Nothing4You just wanted to do a friendly check-in and see how things are going? I see that you have created some tests with haproxy but seems like there are some issues with MySQL 8.0. Let me know if there is anything I can do to help. Thanks for your work on this! |
I haven't had much time to continue troubleshooting that yet unfortunately. I've chosen a socket connection for the test as this simulates mostly the same security level you'd have on a TLS connection, allowing e.g. using auth methods that require a secure connection. I'll be mostly unavailable until the end of the month again, if you want to look into the connection on protocol level, it can probably be reproduced by setting up the MySQL and HAproxy containers and just simulating the PyMySQL connection as used here: |
Thanks for the update!
Yes PyMySQL does work against Cloud SQL with MySQL 8.0. We currently support PyMySQL through the Python Connector with this code. I will look into trying to replicate the issue and see if I can look into it a bit.
I will take a look at the tests and see if I notice anything. Socket connection make the most sense as it is how we connect via PyMySQL currently. |
@Nothing4You Sorry I was on vacation for October! Just wanted to check if you have had any luck here? We would love to get support for I have forked the repository and am currently seeing if I can get to the bottom of the MySQL 8.0 CI/CD issue. |
We faced this problem too. Current solution we have found is to ignore verification: import asyncio
import sqlalchemy as sa
import ssl
from sqlalchemy.ext.asyncio import create_async_engine
async def main():
ctx = ssl.create_default_context()
ctx.check_hostname = False
ctx.verify_mode = ssl.CERT_NONE
engine = create_async_engine(
"mysql+aiomysql://...",
connect_args={"ssl": ctx},
)
conn = await engine.connect()
result = await conn.execute(sa.select(666))
print(result.fetchall())
await conn.close()
await engine.dispose()
asyncio.run(main()) |
Is your feature request related to a problem?
Not currently able to support aiomysql with Cloud SQL Python Connector.
Describe the solution you'd like
The Cloud SQL Python Connector would like to support database connections to Cloud SQL using aiomysql. The Cloud SQL connectors connect to a server side proxy that authorizes users based on a TLS client cert. In order to do this in aiomysql, we require the ability to configure the connection level SSL (outside of the database protocol) or pass in an existing connection (with its own SSL/TLS configuration).
Describe alternatives you've considered
For the pg8000 driver, we use the first option – their
ssl_context
argument allows us to pass in our pre-configured ssl.SSLContext object as long as the customrequire_ssl
attribute is set toFalse
in order to skip the database level SSL protocol . pg8000 codeFor PyMySQL, we create the connection ahead of time, wrap it with our own SSL config, and pass it to the driver.
Additional context
Would either of these options be suitable for aiomysql? Happy to provide more information or assist on this if needed. Thanks so much!
Code of Conduct
The text was updated successfully, but these errors were encountered: