scapy icon indicating copy to clipboard operation
scapy copied to clipboard

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

Open alex opened this issue 1 year ago • 8 comments

Brief description

https://github.com/secdev/scapy/blob/ae79fcba3107ef3cd40aed437fe921a853b6ad6e/scapy/layers/tls/crypto/cipher_block.py#L207-L222

This relies on register_cipher_adapter and GetCipherByName which are not documented public APIs, and will be removed in https://github.com/pyca/cryptography/pull/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

alex avatar Jan 27 '24 13:01 alex

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 (https://github.com/secdev/scapy/pull/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.

gpotter2 avatar Jan 27 '24 18:01 gpotter2

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?

alex avatar Jan 27 '24 21:01 alex

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

reaperhulk avatar Jan 27 '24 22:01 reaperhulk

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.

gpotter2 avatar Jan 27 '24 23:01 gpotter2

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

reaperhulk avatar Jan 30 '24 22:01 reaperhulk

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

gpotter2 avatar Jan 31 '24 08:01 gpotter2

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 😄

reaperhulk avatar Jan 31 '24 15:01 reaperhulk

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

gpotter2 avatar Feb 01 '24 10:02 gpotter2