cryptography icon indicating copy to clipboard operation
cryptography copied to clipboard

Add `SSL_peek_ex`, `SSL_read_ex`, and `SSL_write_ex` to bindings

Open illia-v opened this issue 3 years ago • 4 comments

Since support for OpenSSL < 1.1.1 has been dropped, it looks like it is possible to add these functions to bindings and use them in pyOpenSSL to increase the number of bytes the library allows to read and write before raising OverflowError.

illia-v avatar Sep 08 '22 18:09 illia-v

Don't use the functions, they are buggy and won't get fixed until OpenSSL 4.0. I wasn't aware about the bug when I introduced them in Python. See @davidben 's bug https://github.com/openssl/openssl/issues/8208 for more infos.

tiran avatar Sep 08 '22 19:09 tiran

@tiran interesting, thanks for letting me know. Do you know if SSL_write_ex is buggy?

illia-v avatar Sep 08 '22 19:09 illia-v

Given these don't work on boring/libre I'm reluctant to have us switch to using them in pyOpenSSL. I'm also unclear how much of a performance problem it is to call read/write more often. The current limit is presumably ~2GB, is that a real problem?

reaperhulk avatar Sep 10 '22 16:09 reaperhulk

@reaperhulk I think developers need to call both SSL_read and SSL_read_ex in a loop because OpenSSL always returns around 16 KiB at a time. Regardless of the peculiarity, the functions are usually called with the total number of needed bytes.

The problem is that, as a developer, I would not expect that a program will fail if an integer 231 or greater is passed to a Python function. A program may be tested by downloading/uploading a few kilobytes. Then it may fail in production unexpectedly if it has to deal with a few gigabytes.

illia-v avatar Sep 10 '22 18:09 illia-v

Notwithstanding that using int for length types is unfortunate, the changing/wrong semantics around EOF are sufficient for me to say we don't want to expose/use these functions until they are resolved. There's been no movement from the OpenSSL devs, so for now we aren't going to accept this PR.

Thanks for your contribution though.

alex avatar Oct 12 '22 14:10 alex