Out-of-band data + SSH Agent forwarding

[rinne]

Pluggable out of band communication mechanism over Mosh transport
layer and agent forwarding support in top of out of band mechanism.

I contrubute this code to Mosh project under the same license that
Mosh itself is distributed. All licensing options and clauses
included.

Signed-off-by: Timo J. Rinne tri@iki.fi

[keithw]

This looks great and very robust.

Repeating a bit from IRC:

14:47 < KeithW_> Rinne, re: the OOB protocol -- I'm tending to think it should
be at one level lower in the stack, i.e. at the datagram layer.
14:47 < KeithW_> So that we can do proper flow control and selective ACK and
all that stuff we are going to want to do.
14:55 < Rinne> i don't think i have time and/or energy to put into it in next
few . it has reached a healthy "works for me" -level and also
"not a terrible hack" -level.
14:55 < Rinne> ... next few months ...
14:55 < Rinne> but if someone wants to patch it, i don't mind.
14:56 < KeithW_> All right, thanks. Let me give it some thought.

15:09 < KeithW> Ok. FWIW I think your suggestion to augment our Select class to
include tests for writeable is totally the right call.
15:09 < KeithW> We should just do that.

[rinne]

Some more IRC:

18:59 < Rinne> did i already comment on oob's position in the protocol stack. i remember writing
it but maybe never actually sent to irc.
19:00 < KeithW> Hmm, I don't remember that.
19:01 < Rinne> that i think most logical place for it to be being the one it is now. anyways that
packet exchange has the timeouts calculated automatically and handles throttling and
explicit slow down messages etc.
19:02 < Rinne> even if the volume of oob should grow, it still could live there, if the oob
queuing, acking, and resending is tuned. and that tuning is backwards compatible
anyways.
19:03 < KeithW> My concern is mostly that we do a pretty bad job of handling Instructions bigger
than one segment.
19:03 < KeithW> And we have no real way to do a partial ACK of some datagrams but not others.
19:03 < Rinne> and anyways, take your time. i'm in no hurry beacuse i already have it in my mac :)
19:04 < Rinne> yeah, well, i don't see it as a very big problem.
19:04 < Rinne> and talking about me, i'd be very reluctant to add anything volume intensive to oob.
19:05 < Rinne> mosh is just not a right place for it.
19:05 < KeithW> Well, people do want stuff like arbitrary stream forwarding, sshfs, X11
forwarding...
19:06 < Rinne> i know. but since it's a bad idea, i just don't think about it :)

[djc]

So what's the skinny on this? Could it be included before someone moves it down to the datagram layer?

[robertlacroix]

👍, patch works flawlessly, would be awesome to get it in.

[TomFrost]

Another +1 here -- would be great to get this upstream.

[kenrestivo]

+1 for this too. I'd love to have this feature in the upstream packages.

It looks like a pretty sizable patch though and maybe the maintainers are waiting for some kind of audit of it first before merging it?

[brentley]

adding my +1. I'm running this patch set hoping that it gets merged.

[CheRuisiBesares-OLD]

What is the status of this patch? Would love to have this feature.

[beddari]

I agree with @rinne and as a mosh user I'd like this feature as-is

[alexbw]

Any chance this will make it into the main repo?

[hannes-ucsc]

All right, thanks. Let me give it some thought.

You had a year to mull it over. What's the verdict?

[GothAck]

Any movement on this patch? I see there are now merge conflicts. Mosh supporting OOB data, including ssh-agent, would be an amazingly useful thing to have.

[rinne]

I expected those conflicts to be substantial, because they were mentioned so many times lately. In contrast they appeared to be trivial.

Should anyone be interested to build an agent enabled version of otherwise current keithw master branch stuff, it can be done in the ssh-agent-forwarding branch of my repo https://github.com/rinne/mosh

$ git checkout https://github.com/rinne/mosh.git

$ cd mosh

$ ./autogen.sh && ./configure && make

Agent enabled version must be available in both ends. The current one is fully compatible with the one in this pull request. Agent enabled version is also fully compatible with non-agent-enabled mosh version (except of course the request for agent forwarding is silently ignored by non-agent-enabled server).

I won't bother with another pull request unless someone asks for it.

[GothAck]

@rinne Getting build warning from the merged branch in your repo, was this introduced with your merge of the conflicts? The debuild script treats warnings as errors.

outofband.h:60:9: error: ‘class Network::OutOfBandCommunicator’ has pointer data members [-Werror=effc++]
   class OutOfBandCommunicator
         ^
outofband.h:60:9: error:   but does not override ‘Network::OutOfBandCommunicator(const Network::OutOfBandCommunicator&)’ [-Werror=effc++]
outofband.h:60:9: error:   or ‘operator=(const Network::OutOfBandCommunicator&)’ [-Werror=effc++]
cc1plus: all warnings being treated as errors

Could you perhaps add a merge commit to this branch and push? IIRC that should be added to the PR.

[rinne]

My compilation in OpenSuse or in OSX (under macports) doesn't get that warning and I don't have time to decipher that. Maybe you can fix it and send me a patch against ssh-agent-forwarding branch?

[Kriechi]

@rinne any chance you could rebase and fix the conflicts?
@keithw any final verdict on this PR?

would be a really cool feature to have merged!

[jonlundy]

+1

[rinne]

I probably won't do anything to it, if there is no clear indication that it's going to be merged. Meanwhile the agent enabled version can be compiled from my repo.

repo: https://github.com/rinne/mosh.git
branch: ssh-agent-forwarding

[ryanpetrello]

👍