On 12/29/22 08:24, Priyankar Jain wrote:
> Currently _send() in stream.py[L#415] converts the string buffer to bytes in
> case the caller passes it a string buffer.
>
> This becomes a problem when retrying this send call (say due to EAGAIN errors)
> with SSLStream and underlying OpenSSL implementation.
> Converting the buffer from string to bytes effectively changes the buffer
> address (even though its content might be same).
> But the OpenSSL implementation expects that the buffer contents and address
> to remain same when retrying send/ssl_write on retryable errors such
> as EAGAIN.
>
> Following is the sample stacktrace seen when stream.send() return EAGAIN error
> and the call is retried by jsonrpc.run():
>
> ```
> File "/usr/local/lib/python3.9/site-packages/ovs/db/idl.py", line 1652,
> in commit_block
> self.idl.run()
> File "/usr/local/lib/python3.9/site-packages/ovs/db/idl.py", line 240,
> in run
> self._session.run()
> File "/usr/local/lib/python3.9/site-packages/ovs/jsonrpc.py", line
> 503, in run
> self.rpc.run()
> File "/usr/local/lib/python3.9/site-packages/ovs/jsonrpc.py", line
> 202, in run
> retval = self.stream.send(self.output)
> File "/usr/local/lib/python3.9/site-packages/ovs/stream.py", line 831,
> in send
> return super(SSLStream, self).send(buf)
> File "/usr/local/lib/python3.9/site-packages/ovs/stream.py", line 413,
> in send
> return self.socket.send(buf)
> File "/usr/local/lib/python3.9/site-packages/OpenSSL/SSL.py", line
> 1652, in send
> self._raise_ssl_error(self._ssl, result)
> File "/usr/local/lib/python3.9/site-packages/OpenSSL/SSL.py", line
> 1566, in _raise_ssl_error
> _raise_current_error()
> File "/usr/local/lib/python3.9/site-packages/OpenSSL/_util.py", line
> 57, in exception_from_error_queue
> raise exception_type(errors)
> OpenSSL.SSL.Error: [('SSL routines', 'ssl3_write_pending', 'bad write
> retry')]
> ```
>
> This patch fixes it by maintaining the data to be sent as bytes instead
> of string in jsonrpc.py so that on retries buffer remains unchanged.
>
> Signed-off-by: Priyankar Jain <[email protected]>
Hi. Thanks for the patch!
Though it looks like you're using pyOpenSSL library while OVS is not using
it since 2.17 release.
With which version did you hit the issue?
Python's native ssl library sets the SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER
and SSL_MODE_AUTO_RETRY, if I'm reading that right:
https://github.com/python/cpython/blob/9dee9731663d670652586c929190f227ab56bd8f/Modules/_ssl.c#L837
So, the issue should not be possible on OVS 2.17+.
In any case, proposed patch doesn't seem to fix the described
issue, because nothing prevents us from calling run+send+run.
If the first run will return EAGAIN, send will modify the buffer
and we'll hit the issue on the second run.
Best regards, Ilya Maximets.
> ---
> python/ovs/jsonrpc.py | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/python/ovs/jsonrpc.py b/python/ovs/jsonrpc.py
> index d5127268a..60c631874 100644
> --- a/python/ovs/jsonrpc.py
> +++ b/python/ovs/jsonrpc.py
> @@ -186,7 +186,7 @@ class Connection(object):
> self.stream = stream
> self.status = 0
> self.input = ""
> - self.output = ""
> + self.output = b''
> self.parser = None
> self.received_bytes = 0
>
> @@ -238,7 +238,7 @@ class Connection(object):
> self.__log_msg("send", msg)
>
> was_empty = len(self.output) == 0
> - self.output += ovs.json.to_string(msg.to_json())
> + self.output += ovs.json.to_string(msg.to_json()).encode("utf-8")
> if was_empty:
> self.run()
> return self.status
> @@ -365,7 +365,7 @@ class Connection(object):
> if self.status == 0:
> self.status = error
> self.stream.close()
> - self.output = ""
> + self.output = b''
>
>
> class Session(object):
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
