Skip to content

TLS Stream: TCP transport compatibility fixes

Artem Boldariev requested to merge artem-tls-tcp-compatibility-fixes into main

This merge request contains some fixes targeted at making TLS Stream transport more compatible with TCP in terms of behaviour.

All of these fixes are from the Stream DNS branch (!6707 (merged)) and are required to make the new transport work. However, not a single one is related to the new functionality, but fixes the old functionality. This merge request likely needs to be at least partially backported to 9.18 as well.

1. Fix isc_nm_read_stop() and internal socket read flags handling

It turned out that after the latest Network Manager refactoring sock->reading flag was not processed correctly. Due to this isc_nm_read_stop() might not work as expected because reading from the underlying TCP socket could have been resume in tls_do_bio() regardless of the sock->reading value.

This bug did not seem to cause problems with DoH, so it was not noticed, but Stream DNS has more strict expectations regarding the underlying transport.

Additionally to the above, the `sock->recv_read flag was completely ignored and corresponding logic was completely unimplemented. That did not allow to implement one fine detail compared to TCP: once reading is started, it could be satisfied by one datum reading.

This merge request fixes the issues above.

2. Use the same error codes that TCP stream code does

This commit changes ISC_R_NOTCONNECTED error code to ISC_R_CANCELLED when attempting to start reading data on the shutting down socket in order to make its behaviour compatible with that of TCP and not break the common code in the unit tests.

3. Always handle send callbacks asynchronously on shutting down sockets

This commit ensures that send callbacks are always called from within the context of its worker thread even in the case of shuttigdown/inactive socket, just like TCP transport does and with which TLS attempts to be as compatible as possible.

Edited by Artem Boldariev

Merge request reports