[Twisted-Python] Effects of calling transport.writeSomeData() ?

[Mark Montague]

On 2014-06-21, 10:24, exarkun at twistedmatrix.com wrote:
> > If bypassing the reactor this way is bad, how bad is it and what are 
> the consequences or effects?
>
> This use is untested.  There's no reason to expect it will continue to 
> work with future Twisted releases (or, really, that it fully works 
> now; since `writeSomeData` bypasses the transport's buffering layer, 
> it seems like you're risking an out-of-order or partial send; probably 
> these will only arise under load so you may not have observed them in 
> your testing).

Thanks for the very helpful reply!  This is exactly the sort of thing I 
was hoping to find out.

>> Is there a better way to get a working solution?
>
> The proper way to do this would be for OpenSSH to acknowledge the 
> operation.  At this point you would know it's safe to proceed to the 
> next operation.  Since you didn't mention anything about 
> acknowledgements, I'm guessing there are none.

Correct, the order of events is:

0. client opens OpenSSH socket, sends hello, receives reply
1. user calls the protocol function to create a new OpenSSH session over 
the existing socket
2. client sends MUX_C_ALIVE_CHECK to OpenSSH
3. OpenSSH replies with MUX_S_ALIVE
4. client sends MUX_C_NEW_SESSION to OpenSSH, then immediately forwards 
the file descriptors
5. OpenSSH replies with either MUX_S_SESSION_OPENED, 
MUX_S_PERMISSION_DENIED, or MUX_S_FAILURE.
6. ...session is now created...
7. OpenSSH sends MUX_S_EXIT_MESSAGE when the session has finished
8. client can now close the OpenSSH socket

It would make sense for OpenSSH to acknowledge the MUX_C_NEW_SESSION in 
step 4 before the file descriptors get sent, but I've read the source 
code and OpenSSH does not do this -- it expects the file descriptors to 
be sent as soon as the new session message is sent.


> Since you're already relying on `self.transport.socket.fileno()` and 
> `send1msg` (basically, bypassing the transport abstraction and just 
> doing socket operations yourself) one improvement you could make would 
> be just to rely on that for the whole thing.  Don't use 
> `writeSomeData`. Use `socket.send(command)`.  At least this way you're 
> only relying on being able to treat a transport like a UNIX socket - 
> not on the particulars of the transport's buffering implementation.
>
> A different approach you could take would be to implement this 
> connection sharing feature for Conch.  I can pretty much guarantee 
> it's possible to implement since an older version of Conch actually 
> did implement it. :)  The implementation was removed because it was 
> fragile, complicated, and poorly tested.  It would be great to 
> re-introduce the functionality with a higher quality implementation.

I'll look into what it might take to implement this in a robust way with 
tests for Conch.  I think that's definitely a preferred solution, since 
it sounds like such a feature might be accepted (I wasn't considering 
this initially since Conch is a generic SSH transport, and piggybacking 
on top of an existing OpenSSH socket is a highly-specific thing.)  If I 
can come up with a good way to do this, I'll submit a patch, otherwise 
I'll keep the functionality as it is now, in a standalone class that 
doesn't use Conch at all.


-- 
   Mark Montague
   mark at catseye.org