diff --git a/.github/workflows/pr-pytest-apps.yml b/.github/workflows/pr-pytest-apps.yml index aa2eac4cb..ee1d7c22c 100644 --- a/.github/workflows/pr-pytest-apps.yml +++ b/.github/workflows/pr-pytest-apps.yml @@ -45,7 +45,7 @@ jobs: env: PYTHON_VERSION: ${{ matrix.python-version }} run: | - poetry run pytest --ignore=pynetdicom/tests + poetry run pytest -x --ignore=pynetdicom/tests # pydicom-release: # runs-on: ubuntu-latest diff --git a/.github/workflows/pr-pytest.yml b/.github/workflows/pr-pytest.yml index 3e5311c9d..75be506b0 100644 --- a/.github/workflows/pr-pytest.yml +++ b/.github/workflows/pr-pytest.yml @@ -27,11 +27,14 @@ jobs: run: | conda install pytest conda install -c conda-forge pydicom + - name: Get OpenSSL version + run: | + python -c "import ssl; print('OpenSSL:', ssl.OPENSSL_VERSION_INFO)" - name: Test with pytest env: PYTHON_VERSION: ${{ matrix.python-version }} run: | - pytest --ignore=pynetdicom/apps + pytest -x --ignore=pynetdicom/apps windows: runs-on: windows-latest @@ -65,11 +68,14 @@ jobs: key: venv-${{ runner.os }}-${{ steps.setup-python.outputs.python-version }}-${{ hashFiles('**/poetry.lock') }} - name: Install dependencies run: poetry install --no-interaction --extras tests + - name: Get OpenSSL version + run: | + poetry run python -c "import ssl; print('OpenSSL:', ssl.OPENSSL_VERSION_INFO)" - name: Test with pytest env: PYTHON_VERSION: ${{ matrix.python-version }} run: | - poetry run pytest --cov pynetdicom --ignore=pynetdicom/apps && + poetry run pytest -x --cov pynetdicom --ignore=pynetdicom/apps && poetry run coverage xml - name: Send coverage results if: ${{ success() }} @@ -114,7 +120,7 @@ jobs: env: PYTHON_VERSION: ${{ matrix.python-version }} run: | - poetry run pytest --cov pynetdicom --ignore=pynetdicom/apps && + poetry run pytest -x --cov pynetdicom --ignore=pynetdicom/apps && poetry run coverage xml - name: Send coverage results if: ${{ success() }} @@ -154,11 +160,14 @@ jobs: run: | poetry install --no-interaction --extras tests && poetry run pip list + - name: Get OpenSSL version + run: | + poetry run python -c "import ssl; print('OpenSSL:', ssl.OPENSSL_VERSION_INFO)" - name: Test with pytest env: PYTHON_VERSION: ${{ matrix.python-version }} run: | - poetry run pytest --cov pynetdicom --ignore=pynetdicom/apps && + poetry run pytest -x --cov pynetdicom --ignore=pynetdicom/apps && poetry run coverage xml - name: Send coverage results if: ${{ success() }} diff --git a/docs/changelog/v2.1.0.rst b/docs/changelog/v2.1.0.rst index 558af8cd0..060010a6c 100644 --- a/docs/changelog/v2.1.0.rst +++ b/docs/changelog/v2.1.0.rst @@ -32,4 +32,6 @@ Changes ....... * Dropped support for Python 3.7, 3.8 and 3.9 -* Added support for Python 3.12 +* Added support for Python 3.11 and 3.12 +* With `Python 3.10 requiring OpenSSL v1.1.1 or newer + `_, TLS v1.3 is now officially supported diff --git a/pynetdicom/tests/test_transport.py b/pynetdicom/tests/test_transport.py index 10a0293cb..232a3d45d 100644 --- a/pynetdicom/tests/test_transport.py +++ b/pynetdicom/tests/test_transport.py @@ -172,6 +172,8 @@ def test_close_connect(self): sock.close() assert sock.socket is None # Tries to connect, sets to None if fails + # Ensure we fail if *something* is listening + self.assoc.connection_timeout = 1 request = A_ASSOCIATE() request.called_presentation_address = ("123", 12) sock.connect(T_CONNECT(request)) @@ -312,29 +314,28 @@ def handle_echo(event): assert 2 == len(events) -@pytest.fixture -def server_context(request): - """Return a good server SSLContext.""" - # TLS v1.3 is not currently supported :( - # The actual available attributes/protocols depend on OS, OpenSSL version - # and Python version, ugh - if hasattr(ssl, "TLSVersion"): - # This is the current and future, but heavily depends on OpenSSL - # Python 3.7+, w/ OpenSSL 1.1.0g+ - context = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH) - context.verify_mode = ssl.CERT_REQUIRED - context.load_cert_chain(certfile=SERVER_CERT, keyfile=SERVER_KEY) - context.load_verify_locations(cafile=CLIENT_CERT) - context.maximum_version = ssl.TLSVersion.TLSv1_2 - - return context - - # Should work with older Python and OpenSSL versions - # Python 3.6 - context = ssl.SSLContext(protocol=ssl.PROTOCOL_TLSv1_2) +def server_context_v1_2(): + """Return a good TLs v1.2 server SSLContext.""" + # Python 3.10 and PEP 644, the ssl module requires OpenSSL 1.1.1 or newer + context = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH) + context.verify_mode = ssl.CERT_REQUIRED + context.load_cert_chain(certfile=SERVER_CERT, keyfile=SERVER_KEY) + context.load_verify_locations(cafile=CLIENT_CERT) + context.minimum_version = ssl.TLSVersion.TLSv1_2 + context.maximum_version = ssl.TLSVersion.TLSv1_2 + + return context + + +def server_context_v1_3(): + """Return a good TLS v1.3 server SSLContext.""" + # Python 3.10 and PEP 644, the ssl module requires OpenSSL 1.1.1 or newer + context = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH) context.verify_mode = ssl.CERT_REQUIRED context.load_cert_chain(certfile=SERVER_CERT, keyfile=SERVER_KEY) context.load_verify_locations(cafile=CLIENT_CERT) + context.minimum_version = ssl.TLSVersion.TLSv1_3 + context.maximum_version = ssl.TLSVersion.TLSv1_3 return context @@ -350,6 +351,14 @@ def client_context(request): return context +TLS_SERVER_CONTEXTS = [(server_context_v1_2, "TLSv1.2")] +if ssl.OPENSSL_VERSION_INFO >= (1, 1, 1): + TLS_SERVER_CONTEXTS = [ + (server_context_v1_2, "TLSv1.2"), + (server_context_v1_3, "TLSv1.3"), + ] + + class TestTLS: """Test using TLS to wrap the association.""" @@ -405,7 +414,8 @@ def test_tls_not_server_yes_client(self, client_context): assert len(server.active_associations) == 0 - def test_tls_yes_server_not_client(self, server_context, caplog): + @pytest.mark.parametrize("server_context, tls_version", TLS_SERVER_CONTEXTS) + def test_tls_yes_server_not_client(self, server_context, tls_version, caplog): """Test wrapping the acceptor socket with TLS (and not client).""" with caplog.at_level(logging.ERROR, logger="pynetdicom"): self.ae = ae = AE() @@ -413,7 +423,7 @@ def test_tls_yes_server_not_client(self, server_context, caplog): server = ae.start_server( ("localhost", 11112), block=False, - ssl_context=server_context, + ssl_context=server_context(), ) ae.add_requested_context("1.2.840.10008.1.1") @@ -425,7 +435,10 @@ def test_tls_yes_server_not_client(self, server_context, caplog): assert len(server.active_associations) == 0 assert "Connection closed before the entire PDU was received" in caplog.text - def test_tls_yes_server_yes_client(self, server_context, client_context): + @pytest.mark.parametrize("server_context, tls_version", TLS_SERVER_CONTEXTS) + def test_tls_yes_server_yes_client( + self, server_context, tls_version, client_context + ): """Test associating with TLS on both ends.""" self.ae = ae = AE() ae.acse_timeout = 5 @@ -435,13 +448,14 @@ def test_tls_yes_server_yes_client(self, server_context, client_context): server = ae.start_server( ("localhost", 11112), block=False, - ssl_context=server_context, + ssl_context=server_context(), ) wait_for_server_socket(server, 1) ae.add_requested_context("1.2.840.10008.1.1") assoc = ae.associate("localhost", 11112, tls_args=(client_context, None)) + assert assoc.dul.socket.socket.version() == tls_version assert assoc.is_established assoc.release() assert assoc.is_released @@ -450,7 +464,8 @@ def test_tls_yes_server_yes_client(self, server_context, client_context): assert len(server.active_associations) == 0 - def test_tls_transfer(self, server_context, client_context): + @pytest.mark.parametrize("server_context, tls_version", TLS_SERVER_CONTEXTS) + def test_tls_transfer(self, server_context, tls_version, client_context): """Test transferring data after associating with TLS.""" ds = [] @@ -469,7 +484,7 @@ def handle_store(event): server = ae.start_server( ("localhost", 11112), block=False, - ssl_context=server_context, + ssl_context=server_context(), evt_handlers=handlers, ) @@ -524,7 +539,8 @@ def test_no_ssl_scu(self): with pytest.raises(RuntimeError, match=msg): ae.associate("localhost", 11112, tls_args=(["random", "object"], None)) - def test_multiple_pdu_req(self, server_context, client_context): + @pytest.mark.parametrize("server_context, tls_version", TLS_SERVER_CONTEXTS) + def test_multiple_pdu_req(self, server_context, tls_version, client_context): """Test what happens if two PDUs are sent before the select call.""" events = [] @@ -542,7 +558,7 @@ def handle_echo(event): server = ae.start_server( ("localhost", 11112), block=False, - ssl_context=server_context, + ssl_context=server_context(), ) assoc = ae.associate( @@ -567,7 +583,8 @@ def handle_echo(event): assert assoc.is_released - def test_multiple_pdu_acc(self, server_context, client_context): + @pytest.mark.parametrize("server_context, tls_version", TLS_SERVER_CONTEXTS) + def test_multiple_pdu_acc(self, server_context, tls_version, client_context): """Test what happens if two PDUs are sent before the select call.""" events = [] @@ -585,7 +602,7 @@ def handle_echo(event): server = ae.start_server( ("localhost", 11112), block=False, - ssl_context=server_context, + ssl_context=server_context(), evt_handlers=[(evt.EVT_C_ECHO, handle_echo)], ) diff --git a/pynetdicom/transport.py b/pynetdicom/transport.py index 01d6b5552..fa645c86a 100644 --- a/pynetdicom/transport.py +++ b/pynetdicom/transport.py @@ -14,19 +14,11 @@ _HAS_SSL = True except ImportError: + # NOTE: Must check `_HAS_SSL` before all use of `ssl` module + # and must use "ssl.SSLContext" in type hints _HAS_SSL = False import threading -from typing import ( - TYPE_CHECKING, - Optional, - Any, - Tuple, - cast, - List, - Dict, - Callable, - Union, -) +from typing import TYPE_CHECKING, Any, cast, Callable, Optional from pynetdicom import evt, _config from pynetdicom._globals import MODE_ACCEPTOR, BIND_ADDRESS @@ -79,9 +71,9 @@ def __init__(self, request: "A_ASSOCIATE") -> None: ) @property - def address(self) -> Tuple[str, int]: + def address(self) -> tuple[str, int]: """Return the peer's ``(str: IP address, int: port)``.""" - return cast(Tuple[str, int], self.request.called_presentation_address) + return cast(tuple[str, int], self.request.called_presentation_address) @property def result(self) -> str: @@ -136,8 +128,8 @@ class AssociationSocket: def __init__( self, assoc: "Association", - client_socket: Optional[socket.socket] = None, - address: Tuple[str, int] = BIND_ADDRESS, + client_socket: socket.socket | None = None, + address: tuple[str, int] = BIND_ADDRESS, ) -> None: """Create a new :class:`AssociationSocket`. @@ -163,7 +155,7 @@ def __init__( ) self._ready = threading.Event() - self.socket: Optional[socket.socket] + self.socket: socket.socket | None if client_socket is None: self.socket = self._create_socket(address) @@ -175,7 +167,7 @@ def __init__( # Evt5: Transport connection indication self.event_queue.put("Evt5") - self._tls_args: Optional[Tuple["ssl.SSLContext", str]] = None + self._tls_args: tuple["ssl.SSLContext", str] | None = None self.select_timeout = 0.5 @property @@ -253,7 +245,7 @@ def connect(self, primitive: T_CONNECT) -> None: LOGGER.error("Association request failed: unable to connect to remote") LOGGER.error(f"TCP Initialisation Error: {exc}") # Log exception if TLS issue to help with troubleshooting - if isinstance(exc, ssl.SSLError): + if _HAS_SSL and isinstance(exc, ssl.SSLError): LOGGER.exception(exc) # Don't be tempted to replace this with a self.close() call - @@ -271,7 +263,7 @@ def connect(self, primitive: T_CONNECT) -> None: finally: self._ready.set() - def _create_socket(self, address: Tuple[str, int] = BIND_ADDRESS) -> socket.socket: + def _create_socket(self, address: tuple[str, int] = BIND_ADDRESS) -> socket.socket: """Create a new IPv4 TCP socket and set it up for use. *Socket Options* @@ -315,12 +307,12 @@ def event_queue(self) -> "queue.Queue[str]": """ return self.assoc.dul.event_queue - def get_local_addr(self, host: Tuple[str, int] = ("10.255.255.255", 1)) -> str: + def get_local_addr(self, host: tuple[str, int] = ("10.255.255.255", 1)) -> str: """Return an address for the local computer as :class:`str`. Parameters ---------- - host : Tuple[str, int] + host : tuple[str, int] The host's ``(addr: str, port: int)`` when trying to determine the local address. """ @@ -451,12 +443,12 @@ def __str__(self) -> str: return self.socket.__str__() @property - def tls_args(self) -> Optional[Tuple["ssl.SSLContext", str]]: + def tls_args(self) -> tuple["ssl.SSLContext", str] | None: """Get or set the TLS context and hostname. Parameters ---------- - tls_args : Tuple[ssl.SSLContext, str] or None + tls_args : tuple[ssl.SSLContext, str] or None If the socket should be wrapped by TLS then this is ``(context, hostname)``, where *context* is a :class:`ssl.SSLContext` that will be used to wrap the socket and @@ -466,12 +458,12 @@ def tls_args(self) -> Optional[Tuple["ssl.SSLContext", str]]: Returns ------- - Optional[Tuple[ssl.SSLContext, str]] + tuple[ssl.SSLContext, str] | None """ return self._tls_args @tls_args.setter - def tls_args(self, tls_args: Optional[Tuple["ssl.SSLContext", str]]) -> None: + def tls_args(self, tls_args: tuple["ssl.SSLContext", str] | None) -> None: """Set the TLS arguments for the socket.""" if not _HAS_SSL: raise RuntimeError("Your Python installation lacks support for SSL") @@ -516,14 +508,14 @@ def handle(self) -> None: assoc.start() @property - def local(self) -> Tuple[str, int]: + def local(self) -> tuple[str, int]: """Return a 2-tuple of the local server's ``(host, port)`` address.""" return self.server.server_address @property - def remote(self) -> Tuple[str, int]: + def remote(self) -> tuple[str, int]: """Return a 2-tuple of the remote client's ``(host, port)`` address.""" - return cast(Tuple[str, int], self.client_address) + return cast(tuple[str, int], self.client_address) def _create_association(self) -> "Association": """Create an :class:`Association` object for the current request. @@ -561,7 +553,7 @@ def _create_association(self) -> "Association": if event.is_intervention and self.server._handlers[event]: assoc.bind(event, *self.server._handlers[event]) elif isinstance(event, evt.NotificationEvent): - # List[Tuple[Callable, Optional[List[Any]]]] + # list[tuple[Callable, list[Any] | None]] for handler in self.server._handlers[event]: handler = cast(evt._HandlerBase, handler) assoc.bind(event, handler[0], handler[1]) @@ -593,19 +585,19 @@ class AssociationServer(TCPServer): The parent AE that is running the server. request_queue_size : int Default ``5``. - server_address : Tuple[str, int] + server_address : tuple[str, int] The ``(host: str, port: int)`` that the server is running on. """ def __init__( self, ae: "ApplicationEntity", - address: Tuple[str, int], + address: tuple[str, int], ae_title: str, - contexts: List[PresentationContext], + contexts: list[PresentationContext], ssl_context: Optional["ssl.SSLContext"] = None, - evt_handlers: Optional[List[evt.EventHandlerType]] = None, - request_handler: Optional[Callable[..., BaseRequestHandler]] = None, + evt_handlers: list[evt.EventHandlerType] | None = None, + request_handler: Callable[..., BaseRequestHandler] | None = None, ) -> None: """Create a new :class:`AssociationServer`, bind a socket and start listening. @@ -614,7 +606,7 @@ def __init__( ---------- ae : ae.ApplicationEntity The parent AE that's running the server. - address : Tuple[str, int] + address : tuple[str, int] The ``(host: str, port: int)`` that the server should run on. ae_title : str The AE title of the SCP. @@ -638,8 +630,8 @@ def __init__( self.contexts = contexts self.ssl_context = ssl_context self.allow_reuse_address = True - self.server_address: Tuple[str, int] = address - self.socket: Optional[socket.socket] = None # type: ignore[assignment] + self.server_address: tuple[str, int] = address + self.socket: socket.socket | None = None # type: ignore[assignment] request_handler = request_handler or RequestHandler @@ -649,12 +641,9 @@ def __init__( # Stores all currently bound event handlers so future # Associations can be bound - self._handlers: Dict[ + self._handlers: dict[ evt.EventType, - Union[ - List[Tuple[Callable, Optional[List[Any]]]], - Tuple[Callable, Optional[List[Any]]], - ], + list[tuple[Callable, list[Any] | None]] | tuple[Callable, list[Any] | None], ] = {} self._bind_defaults() @@ -665,7 +654,7 @@ def __init__( self._gc = [0, 59] def bind( - self, event: evt.EventType, handler: Callable, args: Optional[List[Any]] = None + self, event: evt.EventType, handler: Callable, args: list[Any] | None = None ) -> None: """Bind a callable `handler` to an `event`. @@ -706,18 +695,18 @@ def _bind_defaults(self) -> None: self.bind(evt.EVT_PDU_SENT, standard_pdu_sent_handler) @property - def active_associations(self) -> List["Association"]: + def active_associations(self) -> list["Association"]: """Return the server's running :class:`~pynetdicom.association.Association` acceptor instances """ # Find all AcceptorThreads with `_server` as self threads = cast( - List["Association"], + list["Association"], [tt for tt in threading.enumerate() if "AcceptorThread" in tt.name], ) return [tt for tt in threads if tt._server is self] - def get_events(self) -> List[evt.EventType]: + def get_events(self) -> list[evt.EventType]: """Return a list of currently bound events. .. versionadded:: 1.3 @@ -753,7 +742,7 @@ def get_handlers(self, event: evt.EventType) -> evt.HandlerArgType: return self._handlers[event] - def get_request(self) -> Tuple[socket.socket, Tuple[str, int]]: + def get_request(self) -> tuple[socket.socket, tuple[str, int]]: """Handle a connection request. If :attr:`~AssociationServer.ssl_context` is set then the client socket @@ -778,8 +767,8 @@ def get_request(self) -> Tuple[socket.socket, Tuple[str, int]]: def process_request( self, - request: Union[socket.socket, Tuple[bytes, socket.socket]], - client_address: Union[Tuple[str, int], str], + request: socket.socket | tuple[bytes, socket.socket], + client_address: tuple[str, int] | str, ) -> None: """Process a connection request""" # Calls request_handler(request, client_address, self) @@ -886,8 +875,8 @@ class ThreadedAssociationServer(ThreadingMixIn, AssociationServer): def process_request_thread( self, - request: Union[socket.socket, Tuple[bytes, socket.socket]], - client_address: Union[Tuple[str, int], str], + request: socket.socket | tuple[bytes, socket.socket], + client_address: tuple[str, int] | str, ) -> None: """Process a connection request.""" # pylint: disable=broad-except diff --git a/pyproject.toml b/pyproject.toml index b81760db8..f0d0a63cc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -118,6 +118,9 @@ disallow_untyped_defs = true disallow_incomplete_defs = true +[tool.ruff] +include = ["pynetdicom/*.py"] + [tool.ruff.lint] exclude = [ "pynetdicom/apps/tests/*",