On 8/8/22 12:19, Timothy Redaelli wrote:
> On Mon, 8 Aug 2022 11:42:55 +0200
> Ilya Maximets <[email protected]> wrote:
>
>> On 8/8/22 04:27, Miro Tomaska wrote:
>>> Python std library SSLSocket.send does not allow non-zero value for the
>>> optional flag.
>>>
>>> pyOpenSSL was recently switched for the Python standard library ssl module
>>> commit 68543dd523bd00f53fa7b91777b962ccb22ce679 (python: Replace pyOpenSSL
>>> with ssl).
This should be in a 'Fixes:' tag, i.e.
Fixes: 68543dd523bd ("python: Replace pyOpenSSL with ssl.")
>>> Python SSLsocket.send() does not allow non-zero optional flag and it will
>>> explicitly
>>> raise an exception for that. pyOpenSSL did not nothing with this flag but
>>> kept
>>> it to be compatible with socket API.
>>> https://github.com/pyca/pyopenssl/blob/main/src/OpenSSL/SSL.py#L1844
>>
>> Hi, Miro. Thanks for the patch!
>> I'm puzzled though, how come our unit tests are not all failing?
>> I'm sure we have tests for SSL connections with python. And the
>> cited commit was tested with the neutron ml2/ovn tests over SSL
>> before being accepted with no issues observed.
>
> Because this code is only executed when you have an error
> (POLLERR or POLLHUP) and so it's very unlucky you enter this path of
> the code,
OK. Makes sense.
> that should definitely be fixed with your patch.
>
>>>
>>> In addition, expect for ImportError is not necessary anymore as ssl is part
>>> of
>>> the Python standard library. This type of exception should not happen.
>>
>> IIRC, if openssl is not installed in the system, import will fail.
>> Python itself doesn't contain the implementation for SSL protocols,
>> it relies on shared libraries which are not obligatory. So, we,
>> likely, still need an import check.
>
> Yes, you may build python without ssl support (if ssl libraries are not
> available when you build it) and so try is still necessary (they also
> do that in cpython, for example in asyncio
> https://github.com/python/cpython/blob/main/Lib/asyncio/sslproto.py)
>
> So, please do a v2 by restoring the try: / except ImportError: part and
> I'll ack your patch.
Miro, please, also fix flake8 errors which are failing CI:
python/ovs/socket_util.py:183:80: E501 line too long (80 > 79 characters)
Also the patch name seems to be an opposite to what the patch
is actually doing.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
