ronf / asyncssh

AsyncSSH is a Python package which provides an asynchronous client and server implementation of the SSHv2 protocol on top of the Python asyncio framework.
Eclipse Public License 2.0
1.53k stars 150 forks source link

Suppress deprecation warnings from Cyrptography 43.0.0 when importing ARC4 and TripleDES #674

Closed gmuloc closed 3 weeks ago

gmuloc commented 1 month ago

Cryptography 43.0.0 was released on July 20th 2024 and moved two ciphers ARC4 and TripleDES to the decrepit module as per release notes: https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst#4300---2024-07-20

Suggesting the following change to the code to be backward compatible: https://github.com/gmuloc/asyncssh/tree/fix-crypto-43-deprecation-warnings

ronf commented 1 month ago

Thanks for the report!

Looking at the cryptography release notes, it would seem that the changes you propose here for ARC4 and TripleDES also need to be put into place for Blowfish, CAST5, and SEED, which will be moving under cryptography.hazmat.decrepit even sooner than ARC4 and TripleDES.

Now that this new module exists, I'm thinking it might be good to do something more programmatic here to look in both places for ciphers, rather than having to keep making changes on the AsyncSSH side each time a cipher is moved. I'll experiment with this a bit and see if I can find a clean way to do that.

ronf commented 1 month ago

Here's code that I think should work:

diff --git a/asyncssh/crypto/cipher.py b/asyncssh/crypto/cipher.py
index 68badae..23134e3 100644
--- a/asyncssh/crypto/cipher.py
+++ b/asyncssh/crypto/cipher.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2014-2021 by Ron Frederick <ronf@timeheart.net> and others.
+# Copyright (c) 2014-2024 by Ron Frederick <ronf@timeheart.net> and others.
 #
 # This program and the accompanying materials are made available under
 # the terms of the Eclipse Public License v2.0 which accompanies this
@@ -20,22 +20,23 @@

 """A shim around PyCA for accessing symmetric ciphers needed by AsyncSSH"""

+from types import ModuleType
 from typing import Any, MutableMapping, Optional, Tuple
 import warnings

 from cryptography.exceptions import InvalidTag
 from cryptography.hazmat.primitives.ciphers import Cipher, CipherContext
 from cryptography.hazmat.primitives.ciphers.aead import AESGCM
-from cryptography.hazmat.primitives.ciphers.algorithms import AES, ARC4
-from cryptography.hazmat.primitives.ciphers.algorithms import TripleDES
 from cryptography.hazmat.primitives.ciphers.modes import CBC, CTR

-with warnings.catch_warnings():
-    warnings.simplefilter('ignore')
+import cryptography.hazmat.primitives.ciphers.algorithms as _algs
+
+_decrepit_algs: Optional[ModuleType]

-    from cryptography.hazmat.primitives.ciphers.algorithms import Blowfish
-    from cryptography.hazmat.primitives.ciphers.algorithms import CAST5
-    from cryptography.hazmat.primitives.ciphers.algorithms import SEED
+try:
+    import cryptography.hazmat.decrepit.ciphers.algorithms as _decrepit_algs
+except ImportError:
+    _decrepit_algs = None

 _CipherAlgs = Tuple[Any, Any, int]
@@ -140,27 +141,44 @@ def get_cipher_params(cipher_name: str) -> _CipherParams:

 _cipher_alg_list = (
-    ('aes128-cbc',   AES,       CBC,     0, 16, 16, 16),
-    ('aes192-cbc',   AES,       CBC,     0, 24, 16, 16),
-    ('aes256-cbc',   AES,       CBC,     0, 32, 16, 16),
-    ('aes128-ctr',   AES,       CTR,     0, 16, 16, 16),
-    ('aes192-ctr',   AES,       CTR,     0, 24, 16, 16),
-    ('aes256-ctr',   AES,       CTR,     0, 32, 16, 16),
-    ('aes128-gcm',   None,      None,    0, 16, 12, 16),
-    ('aes256-gcm',   None,      None,    0, 32, 12, 16),
-    ('arcfour',      ARC4,      None,    0, 16,  1,  1),
-    ('arcfour40',    ARC4,      None,    0,  5,  1,  1),
-    ('arcfour128',   ARC4,      None, 1536, 16,  1,  1),
-    ('arcfour256',   ARC4,      None, 1536, 32,  1,  1),
-    ('blowfish-cbc', Blowfish,  CBC,     0, 16,  8,  8),
-    ('cast128-cbc',  CAST5,     CBC,     0, 16,  8,  8),
-    ('des-cbc',      TripleDES, CBC,     0,  8,  8,  8),
-    ('des2-cbc',     TripleDES, CBC,     0, 16,  8,  8),
-    ('des3-cbc',     TripleDES, CBC,     0, 24,  8,  8),
-    ('seed-cbc',     SEED,      CBC,     0, 16, 16, 16)
+    ('aes128-cbc',   'AES',       CBC,     0, 16, 16, 16),
+    ('aes192-cbc',   'AES',       CBC,     0, 24, 16, 16),
+    ('aes256-cbc',   'AES',       CBC,     0, 32, 16, 16),
+    ('aes128-ctr',   'AES',       CTR,     0, 16, 16, 16),
+    ('aes192-ctr',   'AES',       CTR,     0, 24, 16, 16),
+    ('aes256-ctr',   'AES',       CTR,     0, 32, 16, 16),
+    ('aes128-gcm',   None,        None,    0, 16, 12, 16),
+    ('aes256-gcm',   None,        None,    0, 32, 12, 16),
+    ('arcfour',      'ARC4',      None,    0, 16,  1,  1),
+    ('arcfour40',    'ARC4',      None,    0,  5,  1,  1),
+    ('arcfour128',   'ARC4',      None, 1536, 16,  1,  1),
+    ('arcfour256',   'ARC4',      None, 1536, 32,  1,  1),
+    ('blowfish-cbc', 'Blowfish',  CBC,     0, 16,  8,  8),
+    ('cast128-cbc',  'CAST5',     CBC,     0, 16,  8,  8),
+    ('des-cbc',      'TripleDES', CBC,     0,  8,  8,  8),
+    ('des2-cbc',     'TripleDES', CBC,     0, 16,  8,  8),
+    ('des3-cbc',     'TripleDES', CBC,     0, 24,  8,  8),
+    ('seed-cbc',     'SEED',      CBC,     0, 16, 16, 16)
 )

-for _cipher_name, _cipher, _mode, _initial_bytes, \
-        _key_size, _iv_size, _block_size in _cipher_alg_list:
-    _cipher_algs[_cipher_name] = (_cipher, _mode, _initial_bytes)
-    register_cipher(_cipher_name, _key_size, _iv_size, _block_size)
+with warnings.catch_warnings():
+    warnings.simplefilter('ignore')
+
+    for _cipher_name, _alg, _mode, _initial_bytes, \
+            _key_size, _iv_size, _block_size in _cipher_alg_list:
+        if _alg:
+            try:
+                _cipher = getattr(_algs, _alg)
+            except AttributeError as exc: # pragma: no cover
+                if _decrepit_algs:
+                    try:
+                        _cipher = getattr(_decrepit_algs, _alg)
+                    except AttributeError:
+                        raise exc from None
+                else:
+                    raise
+        else:
+            _cipher = None
+
+        _cipher_algs[_cipher_name] = (_cipher, _mode, _initial_bytes)
+        register_cipher(_cipher_name, _key_size, _iv_size, _block_size)

It's a little hard to test this right now, as all of the "decrepit" ciphers are still accessible via both cryptography.hazmat.primitives.ciphers.algorithms and cryptography.hazmat.decrepit.ciphers.algorithms for now. They won't actually start to be removed until cryptography 45, with some not being removed until cryptography 48. However, I tried deleting one of the ciphers completely from cryptography.hazmat.primitives.ciphers.algorithms for test purposes and this code did pick the cipher up from cryptography.hazmat.decrepit.ciphers.algorithms instead.

I also tested that this works with older versions of cryptography, where cryptography.hazmat.decrepit.ciphers.algorithms doesn't exist yet.

So far, there doesn't seem to be a separate "decrepit" module for cipher modes (CBC and CTR), so I've left that code untouched.

ronf commented 1 month ago

This is now available as commit a50f9b3 in the "develop" branch.

theunkn0wn1 commented 3 weeks ago

This is now available as commit a50f9b3 in the "develop" branch.

When we can expect a release with this delta?

ronf commented 3 weeks ago

I'll try to see if I can get something out this weekend.

ronf commented 3 weeks ago

This change is now available in AsyncSSH 2.16.0.

gmuloc commented 1 week ago

Thanks a lot @ronf for taking care of this - apologies for my silence I was taking a long overdue break from screens