Support for passing in connection arg or existing SSL connection to aiomysql.connect()

[jackwotherspoon]

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 custom require_ssl attribute is set to False in order to skip the database level SSL protocol . pg8000 code

For 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

• I agree to follow the aio-libs Code of Conduct

[Nothing4You]

Hi,

I believe what you're asking is already possible.

You can pass an SSLContext using the ssl arg, which is then passed on to loop.create_connection.

[Nothing4You]

just leaving this here to link the related issue on cloud-sql-python-connector's side.
GoogleCloudPlatform/cloud-sql-python-connector#216

[Nothing4You]

I think I misunderstood your request at first, as you'd not only need the custom SSLContext but also customize initial TLS negotiation.

API wise, having defer_connect from PyMySQL would allow closer API compatibility.
OTOH, we don't currently expose something like aiomysql.Connection.connect(), so an implementation like SSLContext.request_ssl might be a smaller change to implement.

Do I understand it correctly, that the cloud connector logic would basically replace the current STARTTLS-like implementation with explicit TLS?

[jackwotherspoon]

@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 SSLContext.request_ssl is probably a smaller change to implement. Is this something that you think is feasible within aiomysql? I am more than willing to try and help out on this :)

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!

[Nothing4You]

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?

[jackwotherspoon]

@Nothing4You

Here is where we configure our ssl.SSLContext object in the Cloud SQL Python Connector, it seems pretty similar to what your mock_server testing fixture already looks like.

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!

[jackwotherspoon]

@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 :)

[Nothing4You]

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.
I don't think we could integrate a real Cloud SQL server in the test workflow, so we'd need an alternative implementation for this.

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.

[jackwotherspoon]

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!

[Nothing4You]

Just realized that my terminology was off apparently.
Explicit TLS seems to usually refer to the opposite of what I meant, where you'd have to signal to the server that you want to use TLS on a non-TLS channel, such as STARTTLS, while implicit TLS, which we want to use here, establishes a TLS connection right away without prior protocol-specific preamble.

I've just created #786 as an option for how to implement this.

[jackwotherspoon]

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 _read_bytes. I've seen a few different errors in this spot but I'll paste the main one I'm seeing, a timeout I have set keeps getting hit.

  File "/usr/local/google/home/jackwoth/.pyenv/versions/3.10.1/lib/python3.10/site-packages/google/cloud/sql/connector/aiomysql.py", line 37, in connect
    conn = await aiomysql.connect(user=user, password=passwd, db=db, host=ip_address, port=SERVER_PROXY_PORT, ssl=ctx, implicit_tls=True, **kwargs)
  File "/usr/local/google/home/jackwoth/.pyenv/versions/3.10.1/lib/python3.10/site-packages/aiomysql/connection.py", line 76, in _connect
    await conn._connect()
  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()
  File "/usr/local/google/home/jackwoth/.pyenv/versions/3.10.1/lib/python3.10/site-packages/aiomysql/connection.py", line 1042, in _get_server_information
    packet = await self._read_packet()
  File "/usr/local/google/home/jackwoth/.pyenv/versions/3.10.1/lib/python3.10/site-packages/aiomysql/connection.py", line 602, in _read_packet
    packet_header = await self._read_bytes(4)
  File "/usr/local/google/home/jackwoth/.pyenv/versions/3.10.1/lib/python3.10/site-packages/aiomysql/connection.py", line 650, in _read_bytes
    data = await self._reader.readexactly(num_bytes)
  File "/usr/local/google/home/jackwoth/.pyenv/versions/3.10.1/lib/python3.10/asyncio/streams.py", line 708, in readexactly
    await self._wait_for_data('readexactly')
  File "/usr/local/google/home/jackwoth/.pyenv/versions/3.10.1/lib/python3.10/asyncio/streams.py", line 502, in _wait_for_data
    await self._waiter
asyncio.exceptions.CancelledError

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!

[Nothing4You]

  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 _request_authentication:

https://github.com/Nothing4You/aiomysql/blob/2e2cf5b40473bbe2fdf1b0771fc28e174603a1c0/aiomysql/connection.py#L544

Looks like it's a little more complicated to patch this in.
We should probably skip _get_server_information() in _connect if implicit_tls is given, then catch up on that in _request_authentication after switching to TLS to ensure we still gather this information.

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?

[jackwotherspoon]

I think I'm getting pretty close, I skipped _get_server_information() like you mentioned, but now inside the _request_authentication method there is still a call to _read_packet that is causing me some issues.

Traceback (most recent call last):
  File "/usr/local/google/home/jackwoth/.pyenv/versions/3.10.1/lib/python3.10/site-packages/aiomysql/connection.py", line 552, in _connect
    await self._request_authentication()
  File "/usr/local/google/home/jackwoth/.pyenv/versions/3.10.1/lib/python3.10/site-packages/aiomysql/connection.py", line 846, in _request_authentication
    auth_packet = await self._read_packet()
  File "/usr/local/google/home/jackwoth/.pyenv/versions/3.10.1/lib/python3.10/site-packages/aiomysql/connection.py", line 626, in _read_packet
    raise OperationalError(
pymysql.err.OperationalError: (2013, 'Lost connection to MySQL server during query')

There is mostly likely a step I am missing that I need to add into _request_authentication, I will keep playing around but if you have any ideas let me know? :)

Thanks again for all the help!

[Nothing4You]

I just pushed an update to the PR, can you try again with those changes?

[jackwotherspoon]

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.

Traceback (most recent call last):
  File "/usr/local/google/home/jackwoth/.pyenv/versions/3.10.1/lib/python3.10/site-packages/aiomysql/connection.py", line 652, in _read_bytes
    data = await self._reader.readexactly(num_bytes)
  File "/usr/local/google/home/jackwoth/.pyenv/versions/3.10.1/lib/python3.10/asyncio/streams.py", line 706, in readexactly
    raise exceptions.IncompleteReadError(incomplete, n)
asyncio.exceptions.IncompleteReadError: 0 bytes read on a total of 4 expected bytes

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/local/google/home/jackwoth/.pyenv/versions/3.10.1/lib/python3.10/site-packages/aiomysql/connection.py", line 546, in _connect
    await self._request_authentication()
  File "/usr/local/google/home/jackwoth/.pyenv/versions/3.10.1/lib/python3.10/site-packages/aiomysql/connection.py", line 845, in _request_authentication
    auth_packet = await self._read_packet()
  File "/usr/local/google/home/jackwoth/.pyenv/versions/3.10.1/lib/python3.10/site-packages/aiomysql/connection.py", line 604, in _read_packet
    packet_header = await self._read_bytes(4)
  File "/usr/local/google/home/jackwoth/.pyenv/versions/3.10.1/lib/python3.10/site-packages/aiomysql/connection.py", line 656, in _read_bytes
    raise OperationalError(CR.CR_SERVER_LOST, msg) from e
pymysql.err.OperationalError: (2013, 'Lost connection to MySQL server during query')

[Nothing4You]

Just pushed an update to #786, that seems to work in my local test with a TLS-terminating HAproxy in the middle.
Wanna give that another try?

[jackwotherspoon]

@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?

[Nothing4You]

next steps are primarily updating the test suite for this.
i've updated the checklist in the PR for this.

[jackwotherspoon]

Awesome, just took a look. Is there anything I can help on for the checklist? I'm not too familiar with HAproxy unfortunately.

[jackwotherspoon]

@Nothing4You I have a draft PR for supporting aiomysql with the Cloud SQL Python Connector up. Let me know if there is anything in particular on your checklist I can help with to expedite the next release of aiomysql :)

[Nothing4You]

Hey @jackwotherspoon,
I'll see when I can fit some time in, it shouldn't be too much remaining work.

I've tested with the following haproxy config (needs haproxy.pem with the appropriate cert, can probably be injected similar to how the GH workflow currently injects the cert to the DB server):

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.
with my current agenda i hope to get this released somewhere around late august or earlier.
i would definitely appreciate a review of the entire change when everything is implemented though.

[jackwotherspoon]

@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 aiomysql using the Python connector.

[jackwotherspoon]

@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!

[Nothing4You]

I haven't had much time to continue troubleshooting that yet unfortunately.
For a quick sanity check, could you confirm if the PyMySQL connection against CloudSQL with MySQL 8.0 works as expected?
On my Macbook I can run the entire test suite against MySQL 8.0 (brew) just fine, but I was able to replicate the issue in containers on a Ubuntu VM.
I just haven't had time yet to dig deeper yet where exactly it is breaking.
The test is failing during the PyMySQL connection while preparing the tests, not inside aiomysql, so I'm suspecting it might be related to difference in behavior when the MySQL server thinks it's being accessed via socket while PyMySQL actually talks via TCP/IP to HAproxy.
To be able to properly troubleshoot this I'll have to capture the traffic on the socket connection (after TLS decryption), e.g. via socat or by proxying it over TCP and then sending it to the unix socket, but I haven't had time to capture that yet.
With the decrypted traffic it should be easy to see where the connection currently breaks.

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:

https://github.com/Nothing4You/aiomysql/blob/78ab0ed9a1045916bbb60d8666a9526bde1945b2/tests/conftest.py#L270

[jackwotherspoon]

Thanks for the update!

For a quick sanity check, could you confirm if the PyMySQL connection against CloudSQL with MySQL 8.0 works as expected?

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'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 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.

[jackwotherspoon]

@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 aiomysql added to the Cloud SQL Python Connector as soon as possible so our customers can use it 😄

I have forked the repository and am currently seeing if I can get to the bottom of the MySQL 8.0 CI/CD issue.

[pohmelie]

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())