From 8b6b9c817675dcfa7bf11f66f266246afc7f4d0f Mon Sep 17 00:00:00 2001 From: Jack Wotherspoon Date: Fri, 22 Mar 2024 17:05:30 -0400 Subject: [PATCH 01/12] WIP: check hostname --- google/cloud/alloydb/connector/refresh.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/google/cloud/alloydb/connector/refresh.py b/google/cloud/alloydb/connector/refresh.py index 623a81c..4de956c 100644 --- a/google/cloud/alloydb/connector/refresh.py +++ b/google/cloud/alloydb/connector/refresh.py @@ -86,8 +86,6 @@ def __init__( self.ip_addrs = ip_addrs # create TLS context self.context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) - # update ssl.PROTOCOL_TLS_CLIENT default - self.context.check_hostname = False # force TLSv1.3 self.context.minimum_version = ssl.TLSVersion.TLSv1_3 # add request_ssl attribute to ssl.SSLContext, required for pg8000 driver From 2caa90737f6b1e51326c6b40d30203d8fe54c079 Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Thu, 28 Mar 2024 14:04:14 +0000 Subject: [PATCH 02/12] chore: change test server name --- tests/unit/mocks.py | 2 +- tests/unit/test_async_connector.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/mocks.py b/tests/unit/mocks.py index 51a0bd2..d176dd6 100644 --- a/tests/unit/mocks.py +++ b/tests/unit/mocks.py @@ -113,7 +113,7 @@ def __init__( "PRIVATE": "127.0.0.1", "PUBLIC": "0.0.0.0", }, - server_name: str = "00000000-0000-0000-0000-000000000000.server.alloydb", + server_name: str = "127.0.0.1", cert_before: datetime = datetime.now(timezone.utc), cert_expiry: datetime = datetime.now(timezone.utc) + timedelta(hours=1), ) -> None: diff --git a/tests/unit/test_async_connector.py b/tests/unit/test_async_connector.py index 52ddd77..a7e082e 100644 --- a/tests/unit/test_async_connector.py +++ b/tests/unit/test_async_connector.py @@ -80,7 +80,7 @@ async def test_AsyncConnector_init_ip_type( """ connector = AsyncConnector(credentials=credentials, ip_type=ip_type) assert connector._ip_type == expected - connector.close() + await connector.close() async def test_AsyncConnector_init_bad_ip_type(credentials: FakeCredentials) -> None: From 265f8800b1ee8c24645533b8f583bfad602a196f Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Thu, 28 Mar 2024 14:16:34 +0000 Subject: [PATCH 03/12] chore: add localhost to test SAN --- tests/unit/mocks.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/mocks.py b/tests/unit/mocks.py index d176dd6..6bb94bd 100644 --- a/tests/unit/mocks.py +++ b/tests/unit/mocks.py @@ -96,7 +96,7 @@ def generate_cert( .serial_number(x509.random_serial_number()) .not_valid_before(now) .not_valid_after(expiration) - ) + ).add_extension(x509.SubjectAlternativeName([x509.DNSName("127.0.0.1")])) return cert, key @@ -113,7 +113,7 @@ def __init__( "PRIVATE": "127.0.0.1", "PUBLIC": "0.0.0.0", }, - server_name: str = "127.0.0.1", + server_name: str = "00000000-0000-0000-0000-000000000000.server.alloydb", cert_before: datetime = datetime.now(timezone.utc), cert_expiry: datetime = datetime.now(timezone.utc) + timedelta(hours=1), ) -> None: From 1e4a4d9564721e6e2c13c4e5d7cd4d0be58fcc7b Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Thu, 28 Mar 2024 14:21:14 +0000 Subject: [PATCH 04/12] chore: mark extension as non-critical --- tests/unit/mocks.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/unit/mocks.py b/tests/unit/mocks.py index 6bb94bd..3f109bd 100644 --- a/tests/unit/mocks.py +++ b/tests/unit/mocks.py @@ -96,7 +96,10 @@ def generate_cert( .serial_number(x509.random_serial_number()) .not_valid_before(now) .not_valid_after(expiration) - ).add_extension(x509.SubjectAlternativeName([x509.DNSName("127.0.0.1")])) + ).add_extension( + x509.SubjectAlternativeName([x509.DNSName("127.0.0.1")]), + critical=False, + ) return cert, key From 80a4c235dc0b783be09b4919e6ab8a1ec1d4dfda Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Thu, 28 Mar 2024 14:30:27 +0000 Subject: [PATCH 05/12] chore: fix type --- tests/unit/mocks.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/unit/mocks.py b/tests/unit/mocks.py index 3f109bd..3c54757 100644 --- a/tests/unit/mocks.py +++ b/tests/unit/mocks.py @@ -16,6 +16,7 @@ from datetime import datetime from datetime import timedelta from datetime import timezone +import ipaddress import ssl import struct from typing import Any, Callable, Dict, List, Optional, Tuple @@ -97,7 +98,9 @@ def generate_cert( .not_valid_before(now) .not_valid_after(expiration) ).add_extension( - x509.SubjectAlternativeName([x509.DNSName("127.0.0.1")]), + x509.SubjectAlternativeName( + [x509.IPAddress(ipaddress.ip_address("127.0.0.1"))] + ), critical=False, ) return cert, key From 391cc92acbcadb979194496131453a440d50a339 Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Thu, 28 Mar 2024 14:34:39 +0000 Subject: [PATCH 06/12] chore: add DNSName --- tests/unit/mocks.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/unit/mocks.py b/tests/unit/mocks.py index 3c54757..0f31094 100644 --- a/tests/unit/mocks.py +++ b/tests/unit/mocks.py @@ -99,7 +99,11 @@ def generate_cert( .not_valid_after(expiration) ).add_extension( x509.SubjectAlternativeName( - [x509.IPAddress(ipaddress.ip_address("127.0.0.1"))] + [ + x509.DNSName("localhost"), + x509.DNSName("127.0.0.1"), + x509.IPAddress(ipaddress.ip_address("127.0.0.1")), + ] ), critical=False, ) From 927625f64943b571f9bf54b69d96c519da1da18f Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Mon, 24 Jun 2024 18:49:18 +0000 Subject: [PATCH 07/12] chore: correctly merge main --- .../alloydb/connector/connection_info.py | 3 - google/cloud/alloydb/connector/refresh.py | 119 ------------------ tests/unit/mocks.py | 9 -- 3 files changed, 131 deletions(-) delete mode 100644 google/cloud/alloydb/connector/refresh.py diff --git a/google/cloud/alloydb/connector/connection_info.py b/google/cloud/alloydb/connector/connection_info.py index 7d5338d..6057711 100644 --- a/google/cloud/alloydb/connector/connection_info.py +++ b/google/cloud/alloydb/connector/connection_info.py @@ -57,9 +57,6 @@ def create_ssl_context(self) -> ssl.SSLContext: # create TLS context context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) - # TODO: Set check_hostname to True to verify the identity in the - # certificate once PSC DNS is populated in all existing clusters. - context.check_hostname = False # force TLSv1.3 context.minimum_version = ssl.TLSVersion.TLSv1_3 diff --git a/google/cloud/alloydb/connector/refresh.py b/google/cloud/alloydb/connector/refresh.py deleted file mode 100644 index 4de956c..0000000 --- a/google/cloud/alloydb/connector/refresh.py +++ /dev/null @@ -1,119 +0,0 @@ -# Copyright 2023 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from __future__ import annotations - -import asyncio -from datetime import datetime -from datetime import timezone -import logging -import ssl -from tempfile import TemporaryDirectory -from typing import Dict, List, Optional, Tuple, TYPE_CHECKING - -from cryptography import x509 - -from google.cloud.alloydb.connector.utils import _write_to_file - -if TYPE_CHECKING: - from cryptography.hazmat.primitives.asymmetric import rsa - -logger = logging.getLogger(name=__name__) - -# _refresh_buffer is the amount of time before a refresh's result expires -# that a new refresh operation begins. -_refresh_buffer: int = 4 * 60 # 4 minutes - - -def _seconds_until_refresh( - expiration: datetime, now: datetime = datetime.now(timezone.utc) -) -> int: - """ - Calculates the duration to wait before starting the next refresh. - Usually the duration will be half of the time until certificate - expiration. - - Args: - expiration (datetime.datetime): Time of certificate expiration. - now (datetime.datetime): Current time (UTC) - Returns: - int: Time in seconds to wait before performing next refresh. - """ - - duration = int((expiration - now).total_seconds()) - - # if certificate duration is less than 1 hour - if duration < 3600: - # something is wrong with certificate, refresh now - if duration < _refresh_buffer: - return 0 - # otherwise wait until 4 minutes before expiration for next refresh - return duration - _refresh_buffer - return duration // 2 - - -class RefreshResult: - """ - Manages the result of a refresh operation. - - Holds the certificates and IP address of an AlloyDB instance. - Builds the TLS context required to connect to AlloyDB database. - - Args: - ip_addrs (Dict[str, str]): The IP addresses of the AlloyDB instance. - key (rsa.RSAPrivateKey): Private key for the client connection. - certs (Tuple[str, List(str)]): Client cert and CA certs for establishing - the chain of trust used in building the TLS context. - """ - - def __init__( - self, - ip_addrs: Dict[str, Optional[str]], - key: rsa.RSAPrivateKey, - certs: Tuple[str, List[str]], - ) -> None: - self.ip_addrs = ip_addrs - # create TLS context - self.context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) - # force TLSv1.3 - self.context.minimum_version = ssl.TLSVersion.TLSv1_3 - # add request_ssl attribute to ssl.SSLContext, required for pg8000 driver - self.context.request_ssl = False # type: ignore - # unpack certs - ca_cert, cert_chain = certs - # get expiration from client certificate - cert_obj = x509.load_pem_x509_certificate(cert_chain[0].encode("UTF-8")) - self.expiration = cert_obj.not_valid_after_utc - - # tmpdir and its contents are automatically deleted after the CA cert - # and cert chain are loaded into the SSLcontext. The values - # need to be written to files in order to be loaded by the SSLContext - with TemporaryDirectory() as tmpdir: - ca_filename, cert_chain_filename, key_filename = _write_to_file( - tmpdir, ca_cert, cert_chain, key - ) - self.context.load_cert_chain(cert_chain_filename, keyfile=key_filename) - self.context.load_verify_locations(cafile=ca_filename) - - -async def _is_valid(task: asyncio.Task) -> bool: - try: - result = await task - # valid if current time is before cert expiration - if datetime.now(timezone.utc) < result.expiration: - return True - except Exception: - # suppress any errors from task - logger.debug("Current refresh result is invalid.") - return False diff --git a/tests/unit/mocks.py b/tests/unit/mocks.py index c2a55d9..9ee22d6 100644 --- a/tests/unit/mocks.py +++ b/tests/unit/mocks.py @@ -122,15 +122,6 @@ def generate_cert( .serial_number(x509.random_serial_number()) .not_valid_before(now) .not_valid_after(expiration) - ).add_extension( - x509.SubjectAlternativeName( - [ - x509.DNSName("localhost"), - x509.DNSName("127.0.0.1"), - x509.IPAddress(ipaddress.ip_address("127.0.0.1")), - ] - ), - critical=False, ) if server_cert: cert = cert.add_extension( From 57487ec0e847c60750c61a3f91419954416e12a6 Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Mon, 24 Jun 2024 19:22:35 +0000 Subject: [PATCH 08/12] chore: remove stripping of trailing period --- google/cloud/alloydb/connector/client.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/google/cloud/alloydb/connector/client.py b/google/cloud/alloydb/connector/client.py index 58498c1..9cf37d6 100644 --- a/google/cloud/alloydb/connector/client.py +++ b/google/cloud/alloydb/connector/client.py @@ -129,15 +129,10 @@ async def _get_metadata( resp = await self._client.get(url, headers=headers, raise_for_status=True) resp_dict = await resp.json() - # Remove trailing period from PSC DNS name. - psc_dns = resp_dict.get("pscDnsName") - if psc_dns: - psc_dns = psc_dns.rstrip(".") - return { "PRIVATE": resp_dict.get("ipAddress"), "PUBLIC": resp_dict.get("publicIpAddress"), - "PSC": psc_dns, + "PSC": resp_dict.get("pscDnsName"), } async def _get_client_certificate( From 21783e9362cdba7f42e888b732e07cf6f387bac1 Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Tue, 25 Jun 2024 17:42:07 +0000 Subject: [PATCH 09/12] chore: try removing trailing slash for SSL --- google/cloud/alloydb/connector/connector.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/alloydb/connector/connector.py b/google/cloud/alloydb/connector/connector.py index 1cd20b1..e4325a2 100644 --- a/google/cloud/alloydb/connector/connector.py +++ b/google/cloud/alloydb/connector/connector.py @@ -250,7 +250,7 @@ def metadata_exchange( """ # Create socket and wrap with SSL/TLS context sock = ctx.wrap_socket( - socket.create_connection((ip_address, SERVER_PROXY_PORT)), + socket.create_connection((ip_address.removesuffix("."), SERVER_PROXY_PORT)), server_hostname=ip_address, ) # set auth type for metadata exchange From a830f6777a5bd60b9179c54863247299594330a2 Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Tue, 25 Jun 2024 17:53:08 +0000 Subject: [PATCH 10/12] chore: add back in rstrip --- google/cloud/alloydb/connector/client.py | 7 ++++++- google/cloud/alloydb/connector/connector.py | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/google/cloud/alloydb/connector/client.py b/google/cloud/alloydb/connector/client.py index 9cf37d6..58498c1 100644 --- a/google/cloud/alloydb/connector/client.py +++ b/google/cloud/alloydb/connector/client.py @@ -129,10 +129,15 @@ async def _get_metadata( resp = await self._client.get(url, headers=headers, raise_for_status=True) resp_dict = await resp.json() + # Remove trailing period from PSC DNS name. + psc_dns = resp_dict.get("pscDnsName") + if psc_dns: + psc_dns = psc_dns.rstrip(".") + return { "PRIVATE": resp_dict.get("ipAddress"), "PUBLIC": resp_dict.get("publicIpAddress"), - "PSC": resp_dict.get("pscDnsName"), + "PSC": psc_dns, } async def _get_client_certificate( diff --git a/google/cloud/alloydb/connector/connector.py b/google/cloud/alloydb/connector/connector.py index e4325a2..1cd20b1 100644 --- a/google/cloud/alloydb/connector/connector.py +++ b/google/cloud/alloydb/connector/connector.py @@ -250,7 +250,7 @@ def metadata_exchange( """ # Create socket and wrap with SSL/TLS context sock = ctx.wrap_socket( - socket.create_connection((ip_address.removesuffix("."), SERVER_PROXY_PORT)), + socket.create_connection((ip_address, SERVER_PROXY_PORT)), server_hostname=ip_address, ) # set auth type for metadata exchange From 35d98f5951f84af3ea5688bae725b7cf630ebb4e Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Tue, 25 Jun 2024 17:57:55 +0000 Subject: [PATCH 11/12] chore: test adding period to hostname --- google/cloud/alloydb/connector/connector.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/alloydb/connector/connector.py b/google/cloud/alloydb/connector/connector.py index 1cd20b1..8044455 100644 --- a/google/cloud/alloydb/connector/connector.py +++ b/google/cloud/alloydb/connector/connector.py @@ -251,7 +251,7 @@ def metadata_exchange( # Create socket and wrap with SSL/TLS context sock = ctx.wrap_socket( socket.create_connection((ip_address, SERVER_PROXY_PORT)), - server_hostname=ip_address, + server_hostname=ip_address + ".", ) # set auth type for metadata exchange auth_type = connectorspb.MetadataExchangeRequest.DB_NATIVE From 38e58b778aca63397a2ed652e3f3bf1d358f3181 Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Tue, 25 Jun 2024 18:00:16 +0000 Subject: [PATCH 12/12] chore: undo test --- google/cloud/alloydb/connector/connector.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/alloydb/connector/connector.py b/google/cloud/alloydb/connector/connector.py index 8044455..1cd20b1 100644 --- a/google/cloud/alloydb/connector/connector.py +++ b/google/cloud/alloydb/connector/connector.py @@ -251,7 +251,7 @@ def metadata_exchange( # Create socket and wrap with SSL/TLS context sock = ctx.wrap_socket( socket.create_connection((ip_address, SERVER_PROXY_PORT)), - server_hostname=ip_address + ".", + server_hostname=ip_address, ) # set auth type for metadata exchange auth_type = connectorspb.MetadataExchangeRequest.DB_NATIVE