ronf / asyncssh

AsyncSSH is a Python package which provides an asynchronous client and server implementation of the SSHv2 protocol on top of the Python asyncio framework.
Eclipse Public License 2.0
1.54k stars 151 forks source link

support for alternative asynchronous event loops #208

Closed asteven closed 3 years ago

asteven commented 5 years ago

If you at some point would want to switch to async/await syntax then please consider supporting alternative event loops like curio and trio. e.g. by using anyio.

;-)

ronf commented 5 years ago

Thanks - I'll take a look.

Unfortunately, I'm not sure how quickly I'll be migrating AsyncSSH away from the original Python 3.4 syntax. I looked recently at my downloads on PyPI broken out by Python version, and there was still a large number of downloads from 3.4 clients. While I'd love to be able to use the much nicer syntax in AsyncSSH itself and completely remove all the old yield calls, I don't want to break things for users of 3.4.

asteven commented 5 years ago

Alternatively maybe consider to implement the SSH protocol as an sans-io protocol. Then it could be used by anybody in python land. It would be the definitive answer to the ssh protocol question.

But I have no clue if this would be possible at all or how much work that would be.

ronf commented 5 years ago

Interesting concept, but I think it misses one of the key points here, which is what I/O abstraction you provide to your caller. While that could also be written in the same sans-io style with any data going into or out of an SSH connection/session being passed as memory buffers, that would make event management like blocking due to flow control all the responsibility of the caller, eliminating much of the benefit that led to my converting AsyncSSH from asyncore to asyncio, providing more than just simple callbacks whenever data needed to be delivered.

I would still like at some point to provide a shim layer for other async I/O paradigms like curio or trio, if someone else doesn't find a way to do something like what "uvloop" did by mimicking the asyncio API for them, but there I'd probably want to redesign my calling API to look more like the abstraction those event loops provide at the TCP layer to make AsyncSSH feel more "native" in them, and I'm not really sure how much work that would be. I definitely need to get more familiar with these other frameworks before attempting something like that.

oremanj commented 5 years ago

Trio developer here -- AsyncSSH is awesome, thanks for all the work you've put into it!

I haven't done exhaustive testing, but simple AsyncSSH cases seem to work well enough atop Trio using https://github.com/python-trio/trio-asyncio currently (this is approximately the "what uvloop did" you were looking for). It would be nice to have a more "native" feeling interface, but it's plenty usable without one.

anyio is definitely cool, but has some downsides:

I'm not sure the effort of switching to anyio would be worth the marginal improvement over the existing trio-asyncio experience for Trio users. (There's no curio-asyncio library yet, and I'm not sure how possible it is to make one since so many things in Curio are async-colored, so it would make more of a difference for Curio users. I don't know how many Curio users are trying to do SSH stuff, though.)

It seems like a better direction to go in the long run might be to factor out the SSH protocol parts of AsyncSSH into a sans-io library, which AsyncSSH and hypothetical TrioSSH/CurioSSH/... projects could all depend on. (It's not out of the question that Paramiko could use it too, though I don't know anything about their development priorities.) You're definitely right that a sans-io SSH implementation doesn't give you a usable SSH library in itself, but I think you might be underestimating how much easier it could make it to write a usable SSH library.

If this seems abstractly desirable to you, even if you don't want to do the work of splitting it up, I'd be interested in hearing so -- it's something I'd be interested in working on myself, though not in the particularly near future.

njsmith commented 5 years ago

I don't know what the right thing to do on any of this is, but here's a few thoughts that you might find relevant:

Unfortunately, I'm not sure how quickly I'll be migrating AsyncSSH away from the original Python 3.4 syntax. I looked recently at my downloads on PyPI broken out by Python version, and there was still a large number of downloads from 3.4 clients. While I'd love to be able to use the much nicer syntax in AsyncSSH itself and completely remove all the old yield calls, I don't want to break things for users of 3.4.

Looking at pypistats.org, I'm seeing a spike in 3.4 downloads between ~Jan-April, but it's dropped off again now to ~3-5%. Hopefully it stays low, since 3.4 is no longer getting security fixes... and in case you didn't know, you can use the python_requires option in setup.py to hide new releases from 3.4 downloaders – if the new version has python_requires=">= 3.5", then 3.4 users will silently get the old version.

While that could also be written in the same sans-io style with any data going into or out of an SSH connection/session being passed as memory buffers, that would make event management like blocking due to flow control all the responsibility of the caller, eliminating much of the benefit that led to my converting AsyncSSH from asyncore to asyncio, providing more than just simple callbacks whenever data needed to be delivered.

Yeah, SSH's flow control stuff would definitely complicate the API of a sans-io library. It's not impossible, though – HTTP/2 has similar flow-control, and h2 library that started this whole sans-io trend exposes that as part of its API.

FWIW, my experience of working on a few sans-io libraries now has been that they don't solve all the problems, but they have led to nicer code overall. (In particular, testing and fuzzing become extremely easy.)

ronf commented 5 years ago

@oremanj Thanks for the pointer to trio-asyncio, and for the general advice about anyio. I did look into Trio a bit when it was fairly new and liked what I saw, but other development work got in the way of my really learning it deeply or figuring out what a "native" Trio version of the AsyncSSH APIs would look like. I'm glad to hear that you were able to get something working with trio-asyncio, though -- that should give Trio users at least one way to leverage AsyncSSH.

I've been thinking about what a sans-io library might look like here, and I can sort of imagine parts of it, but when I look at some of the higher-level APIs like the SSHProcess class which supports automatic I/O redirection from a wide variety of other device types (sockets, files, pipes, etc.), things get a lot more complicated. There would also be challenges with things like the Pageant support on Windows which accesses a shared memory buffer using native Windows APIs. Even something simpler like the UNIX domain socket support used to connect to the OpenSSH agent would not be pretty, as it would go from being a simple RPC-like blocking API to a messaging mechanism that operates on serialized request/response data.

@njsmith Thanks for the pointer to pypistats.org. I had previously found the pypinfo program and even set up a Google BigQuery account to play with it, but I hadn't realized that there was already a site online which published some of these stats.

Even at only ~5% of the downloads, I'd be hesitant to drop Python 3.4 support completely, but I have definitely considered releasing a "2.0" version of AsyncSSH where I drop Python 3.4 support and change all of code in AsyncSSH over to the new async/await syntax, keeping around a "1.x" branch which I could still provide critical fixes on but probably not promise to back-port any of the new features to.

The "python_requires" option seems like it could fit well with the notion of a "2.0" release.

I definitely haven't ruled out eventually doing some form of sans-io library, but with all of the types of I/O that would need to be supported that don't directly related to any of the SSH parsing, I wonder if there might be other ways to abstract things which would still allow a relatively seamless conversion from one async event loop to another without trying to turn everything into calls that operate solely on byte arrays with all the flow control handled in some application-aware way.

njsmith commented 5 years ago

when I look at some of the higher-level APIs like the SSHProcess class which supports automatic I/O redirection from a wide variety of other device types (sockets, files, pipes, etc.), things get a lot more complicated.

I'm not 100% sure I understand what SSHProcess is, but I think if I were writing a sans-io SSH library, I'd probably leave high-level redirection helpers to the I/O-ful wrapper, rather than trying to handle them directly in the sans-io layer. This is pretty common with sans-io wrappers: e.g. h11 handles all the wire-level stuff for implementing HTTP/1.1, but someone else still has to handle any logic for redirects, handling different content-types, cookies, etc. etc. Again, sans-i/o doesn't solve everything :-). It's just a way of sectioning off your core networking code to make it easier to test and re-use.

Trio's way of working with processes is really different from asyncio's (e.g. we don't have protocols or transports!), so a more Trio-oriented wrapper might approach this very differently...

There would also be challenges with things like the Pageant support on Windows which accesses a shared memory buffer using native Windows APIs. Even something simpler like the UNIX domain socket support used to connect to the OpenSSH agent would not be pretty, as it would go from being a simple RPC-like blocking API to a messaging mechanism that operates on serialized request/response data.

This does sound tricky! From a quick look at the asyncssh's agent code, it's definitely not trivially portable to Trio, because it's heavily based around asyncio transport APIs, while Trio uses a totally different abstraction for byte streams. I don't actually know if portability to Trio is something you care about, so this isn't a criticism, just saying that if you do want to make asyncssh more portable then this might be a problem that needs some kind of solution.

You might be interested in this comment (and the overall thread): https://github.com/python-trio/trio/issues/796#issuecomment-504746835

ronf commented 5 years ago

I'm not 100% sure I understand what SSHProcess is, but I think if I were writing a sans-io SSH library, I'd probably leave high-level redirection helpers to the I/O-ful wrapper, rather than trying to handle them directly in the sans-io layer. This is pretty common with sans-io wrappers: e.g. h11 handles all the wire-level stuff for implementing HTTP/1.1, but someone else still has to handle any logic for redirects, handling different content-types, cookies, etc. etc. Again, sans-i/o doesn't solve everything :-). It's just a way of sectioning off your core networking code to make it easier to test and re-use.

The SSHProcess class in AsyncSSH is basically intended to mirror asyncio.subprocess.Process and the non-async subprocess.Process class, except that instead of running the command locally it runs it on a remote system using SSH. It has the same capability to pass in various kinds of file, pipe, socket, or other stream objects as stdin/stdout/stderr, though, including the stdin/stdout/stderr of other SSHProcess objects so that you can chain things. Once a set of these objects are instantiated, a whole bunch of I/O can happen without any explicit calls from the application other than to potentially wait for this I/O to complete or to collect up output in a string if it wasn't already redirected to something like a file.

I agree with you that an interface like this doesn't really fit in the sans-io layer, and would instead need to be built on top. I'm just not sure how much WOULD fit into that layer. I can certainly imagine replacing the state machine I have for breaking a byte stream into SSH "packets", transforming a receive buffer of raw bytes into a buffer of "events" that correspond to SSH packet types, each with accessors for the arguments present in those packets, but that's actually a very tiny portion of the total code. Most of the complexity is in what happens after you have a parsed version of these packets. The SSH packetization code is only a couple of hundred lines, and even adding in code elsewhere that actually assembles and disassembles the packets, I think we're talking about less than 5% of the current AsyncSSH code.

That said, if I could find a way to abstract the actual I/O from everything above it (not just the packet parsing/generation but all of the SSH state machine code that exchanges these packets), I think that would be worthwhile. The challenge is what the top-most API should look like to the application code, though. As you pointed out, the code I have today intentionally mimics the native asyncio low-level transport/protocol callback pattern, the higher-level asyncio "stream" pattern, and some other common patterns like the original Python subprocess callback pattern and the asyncio stream version of that.

I wouldn't mind offering other patterns here, including possibly something trio-like once I understood it better, but I think the real value if I were to do that would be to provide application code with an API for an SSH session/channel that mimicked the API trio provides for a plain TCP socket, so that someone familiar with opening a TCP socket using trio could easily adapt that code to run over SSH instead. Getting AsyncSSH to actually use the trio event loop might be important so an application writer could mix SSH and TCP I/O in a single process, but supporting the trio event loop while keeping an asyncio-style transport/protocol or stream API might not be that interesting/useful.

ronf commented 5 years ago

From a quick look at the asyncssh's agent code, it's definitely not trivially portable to Trio, because it's heavily based around asyncio transport APIs, while Trio uses a totally different abstraction for byte streams.

Actually, the AsyncSSH agent code is given "reader" and "writer" objects to do all its I/O, which look like they map pretty well to trio's ReceiveStream and SendStream objects. The Win32 agent code refers to something it calls a "transport", but it's actually more like a trio bidirectional Stream object, implementing both read and write functions as coroutines (though that Windows implementation doesn't truly do async I/O right now, cheating and using blocking calls but not expecting them to block for long since the agent is on the same machine and the messages are short).

That said, the main SSH connection and channel code does rely on the callback-based API in asyncio, and that would probably be hard to port to trio. This is historical -- the first version of this code was based on asyncore, which only supported callbacks. Things I've written since such as SFTP use the asyncio stream APIs which would be more easily ported to trio, but supporting both asyncio and trio might require ripping out all use of the callback-based APIs and relying only on the stream APIs for the underlying I/O. I'd then need to decide whether I also drop support for the callback-based APIs I provide to application code or not. I suspect I could keep my application callback API support even if I didn't use that on the actual network sockets, but that's something I'd have to look more closely at.

njsmith commented 5 years ago

h11 and h2 both have non-trivial protocol state machines, which might be interesting to compare to: ref1, ref2

Figuring out what this kind of high-level state machine should look like, and implementing it without blocking, is definitely the hardest part of building a complex sans-io library. Without having looked I'm guessing it might take some non-trivial refactoring of your code, if its state machines are written directly against the 3.4-era asyncio callback API. I guess any support for non-asyncio loops will also require similar refactoring, though.

In case it's useful for reference, here's how I'd imagine a fully Trio-native ssh implementation might look:

It would let you run SSH over arbitrary trio.abc.Stream implementations. (Plus some trivial convenience helpers to make-a-TCP-connection-then-run-SSH-over-it, or listen-for-incoming-TCP-connections-then-run-SSH-over-them.)

I'd expect the ssh object to have async methods to create new channels with various configurations, and those channels would generally be represented as some kind of Stream or maybe an object holding several Streams (like trio.Process).

Most importantly: Trio's main radical difference from asyncio is that all concurrency is explicit and bounded – you can't just spawn a background task anywhere you want, there has to be an async with block marking its lifetime. And data doesn't flow unless there's a task pumping it – we don't have anything like protocols/transports that move data around autonomously.

This is crucial b/c SSH is a multiplexed protocol where multiple logically distinct channels can share the same underlying connection, so you need some background tasks to handle reading and writing to the underlying connection, and dispatching to the tasks that are using individual channels. And that means that every SSH connection will need a mandatory async with block to manage the background tasks – probably the convenience helper will look like async with open_ssh(host, port) as ssh: ...

A full sans-io state machine like h2 is flexible enough to be driven by either callbacks like asyncio, or manager tasks like this mode (generally the "background tasks" end up doing all the state-machine stuff). Or, like you say, you can imagine writing it all with async/await, and swap out the stream implementations underneath... but I'm guessing it's a lot easier to do that by writing it in the "native Trio" style and then adding asyncio support, rather than vice-versa. You can implement nurseries on asyncio and asyncio doesn't really have a standard stream abstraction in the same way Trio does, so it's easier to adapt the Trio stuff to asyncio-land than vice-versa.

ronf commented 5 years ago

@njsmith Thanks for the pointers!

I see the value in Trio's concept of nurseries to manage background tasks, but does this mean that pretty much every top-level API call in AsyncSSH that might need to start a new background task would need to take an explicit "nursery" argument? I suppose it could be argued that this is not too different than the way you sometimes pass "loop" arguments explicitly in asyncio, but a key difference is there's no equivalent to asyncio.get_event_loop() here, to automatically get the "current' event loop. You mention a convenience helper function here to create the nursery under the covers and act as a context manager, but it seems like that wouldn't allow multiple unrelated SSH connections to be opened simultaneously with such a helper. Instead, an application wanting to open multiple connections would have to open its own nursery and pass that nursery as an explicit argument to the AsyncSSH calls to open new connections.

Is there a standard pattern for this already in Trio? I don't see any sign of a nursery argument in trio.open_tcp_stream(), but without that how would such a function be able to pump data? I can see how perhaps trio would leave the pumping to the application code to do explicitly in that case, but what about something like trio.open_ssl_over_tcp_stream()? It also doesn't take a nursery, but there are times when SSL needs to do bi-directional I/O on the TCP socket even when the application might not be actively reading or writing (like an SSL key renegotiation), and as you said a multiplexed protocol like SSH definitely needs to be reading streams in the background and cannot rely on any application level code to trigger data pumping.

njsmith commented 5 years ago

I see the value in Trio's concept of nurseries to manage background tasks, but does this mean that pretty much every top-level API call in AsyncSSH that might need to start a new background task would need to take an explicit "nursery" argument?

Maybe. Only if the background task needs to outlive the call. And the usual API convention would be that the user writes:

async with open_ssh(host) as ssh_connection:
    ...

...which opens a nursery internally, and then later on calls like ssh_connection.whatever(...) can use that nursery to spawn tasks, as long as the task lifetime doesn't need to outlive the connection itself.

You mention a convenience helper function here to create the nursery under the covers and act as a context manager, but it seems like that wouldn't allow multiple unrelated SSH connections to be opened simultaneously with such a helper. Instead, an application wanting to open multiple connections would have to open its own nursery and pass that nursery as an explicit argument to the AsyncSSH calls to open new connections.

Right, for folks who need to support a dynamic collection of connections, and each connection needs its own background task, the usual pattern is to also have a lower-level API like:

ssh_connection = SSHConnection(host, nursery)

Then open_ssh is a small wrapper around this. There's some more discussion here: https://stackoverflow.com/questions/48282841/in-trio-how-can-i-have-a-background-task-that-lives-as-long-as-my-object-does/48300537#48300537

I don't see any sign of a nursery argument in trio.open_tcp_stream(), but without that how would such a function be able to pump data?

Excellent question! The answer is, it doesn't pump data :-). The style is more similar to synchronous or threaded programming, where you call methods to read and write data (though of course the implementation doesn't use threads).

what about something like trio.open_ssl_over_tcp_stream()? It also doesn't take a nursery, but there are times when SSL needs to do bi-directional I/O on the TCP socket even when the application might not be actively reading or writing (like an SSL key renegotiation), and as you said a multiplexed protocol like SSH definitely needs to be reading streams in the background and cannot rely on any application level code to trigger data pumping.

For TLS, we do all the pumping of the underlying transport inside calls to ssl_stream.send_all and ssl_stream.receive_some. Like you cleverly noticed, this is tricky to do correctly if you want to handle both renegotiation and full-duplex communication, because you might have one task calling ssl_stream.send_all and another task calling ssl_stream.receive_some at the same time, and then they need to coordinate who's going to pump the transport. You can read the gory details here if you're curious: https://github.com/python-trio/trio/blob/master/trio/_ssl.py

We can just barely get away with this for TLS. For SSH there's no hope, so you just have to create some tasks at the same time the connection is created, with the same lifetime.

ronf commented 4 years ago

Just a quick update here -- I just released AsyncSSH 2.0.0 which is now converted over to the modern Python async/await syntax for everything internally, now requiring Python 3.6 or later. I've created a support branch for AsyncSSH 1.x just in case I need to provide bug fixes for folks on Python 3.4 and 3.5, but my plan is that new feature development will all be on AsyncSSH 2.x.

I haven't gotten back to looking at anyio/sans-io yet, but this is a step toward making that possible.

ronf commented 4 years ago

I started looking at what primitives I use in asyncio today, and what their equivalents might be in trio. I found a way to map most of what I use over, but there were a few things I wasn't sure about. I'd appreciate any suggestions on these:

dolamroth commented 1 year ago

UNIX socket listeners are now available in trio and anyio: https://anyio.readthedocs.io/en/stable/networking.html#working-with-unix-sockets