Skip to content
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

tls/crypto /cipher_block.py relies on cryptography internal details that are going away eventually #4237

Closed
alex opened this issue Jan 27, 2024 · 8 comments · Fixed by #4285

Comments

@alex
Copy link
Contributor

alex commented Jan 27, 2024

Brief description

if GetCipherByName(_gcbn_format)(backend, _ARC2, modes.CBC) != \
backend._ffi.NULL:
class Cipher_RC2_CBC(_BlockCipher):
pc_cls = _ARC2
pc_cls_mode = modes.CBC
block_size = 8
key_len = 16
class Cipher_RC2_CBC_40(Cipher_RC2_CBC):
expanded_key_len = 16
key_len = 5
backend.register_cipher_adapter(Cipher_RC2_CBC.pc_cls,
Cipher_RC2_CBC.pc_cls_mode,
GetCipherByName(_gcbn_format))

This relies on register_cipher_adapter and GetCipherByName which are not documented public APIs, and will be removed in pyca/cryptography#9859

This PR is expected to land in May, so there's a bit of time to resolve.

If there's a missing API in cryptography, please let us know so we can find a proper solution.

Scapy version

main

Python version

all

Operating system

all

Additional environment information

No response

How to reproduce

n/a

Actual result

No response

Expected result

No response

Related resources

No response

@gpotter2
Copy link
Member

gpotter2 commented Jan 27, 2024

Thanks a lot for the notice, that's greatly appreciated.

To give some context, I was also planning to use those two functions in an upcoming PR (#4214).

Because we are a networking library, and network is old, we have some use cases where we want to be able to use and access old and obsolete cryptography algorithms that are still used in the real world in legacy applications. Most of those are still present in OpenSSL but not exported/accessible by cryptography, which is why we were using the already pretty great internal cryptography bindings. Do you know if it will still be possible to achieve a similar result after the change?

As an example, DES-CBC, which is of course completely obsolete, is still supported by MIT Kerberos servers, Heimdal (in samba or without) (and Windows) and it is therefore currently still present in OpenSSL (through legacy providers, if not by default). It is useful for us to have access to it, for the sake of testing legacy products or servers and/or for offensive purposes, etc.

I understand cryptography is aiming not to spread the usage of those legacy algorithms, but it would be really nice for our use case if there was some sort of undocumented way to still use cryptography in those use cases. Our alternatives are not great: in addition of using cryptography for most usages, to support those crappy legacy algorithms we would have to either use PyCryptodome (which is in a terrible state compared to cryptography), other OpenSSL bindings (which is far more annoying, and a bit too bad considering the work already put into cryptography's ones) or pack a custom Python implementation of those algorithms (which we already do for MD4 for instance, and is by far the worse option).

Sorry for the long post.

@alex
Copy link
Contributor Author

alex commented Jan 27, 2024

In the specific case of DES, you can do this with cryptography using TripleDES -- TripleDES behaves the same as single DES for the case of a small key.

For the case here of RC2, I think we'd be willing to add a module for "super bad legacy stuff you should never use" that contains it.

Are there other cases we should consider?

@reaperhulk
Copy link
Contributor

The decrepit module probably is where we'd eventually move the ciphers we currently have marked as deprecated (CAST5, IDEA, SEED, et al).

@gpotter2
Copy link
Member

I think we'd be willing to add a module for "super bad legacy stuff you should never use" that contains it.

That would work great in our case, thanks a lot. I don't currently have any other case in mind, but having a "bad legacy module" would allow us to PR "easily" if we ever get the case, which sounds good.

The decrepit module probably is where we'd eventually move the ciphers we currently have marked as deprecated

That's great to know, it would also work great for us, as long as they remain somewhere.

In the specific case of DES, you can do this with cryptography using TripleDES

I for some reason thought TripleDES would enforce a 112/168 key length, but it indeed appears 56 is also usable. Tested it and it works great, thanks a lot for the tip.

@reaperhulk
Copy link
Contributor

reaperhulk commented Jan 30, 2024

@gpotter2 I've been looking at this a bit more and RC2 is...quite an algorithm. It looks like you only currently implement/test against RC2-128, although you have an unused RC2_40_CBC class. Would it be enough if we gave you RC2-128 only (no alternate key sizes, no effective key bits support), or do you want/need RC2-40 (or RC2-64 for that matter)? And, if you need those, do you need the concept of effective key bits? If so, can we constrain the set of allowable values to something more than the sum of all possible permutations?

@gpotter2
Copy link
Member

gpotter2 commented Jan 31, 2024

Hi @reaperhulk. I believe that we only use RC2 in context of the TLS/SSL ciphers, in which case only the RC2-128 version is used. When the 40bits-key is used (in 'EXPORT' ciphers), it is first derived into 128 bits with a TLS-specific process (that varies depending on whether RC2 is used with SSLv2, SSLv3 or TLS 1.0) that we implement, so there's no need to add support for the per-RC2-spec effective key bits and/or variable lengths.

This class is poorly named, it would have been much more obvious with its actual per-spec name: SSL_CK_RC2_128_CBC_EXPORT40_WITH_MD5

@reaperhulk
Copy link
Contributor

Okay, then I think we'll land something that enables RC2-128-CBC only and then I'll send a PR that tries to import from the new namespace and falls back to your current patch method for older versions of cryptography 😄

@gpotter2
Copy link
Member

gpotter2 commented Feb 1, 2024

Sounds great, thanks a lot ! If you need help with the PR please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants