Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement streamlocal-forward for remote => local UDS forwarding #312

Merged
merged 10 commits into from
Aug 20, 2024

Conversation

kanpov
Copy link
Contributor

@kanpov kanpov commented Jul 6, 2024

I left a // NEED HELP comment on places where I didn't fully figure out what to do, so I'd really appreciate it if some maintainers helped me out in those places.

@kanpov
Copy link
Contributor Author

kanpov commented Jul 6, 2024

I seem to have implemented most if not all of the needed behavior (but not tests yet), but this needs thorough review by people who know the library better than me.

@kanpov
Copy link
Contributor Author

kanpov commented Jul 6, 2024

@Eugeny would really appreciate it if you took a look

russh/src/server/session.rs Outdated Show resolved Hide resolved
russh/src/parsing.rs Outdated Show resolved Hide resolved
russh/src/client/session.rs Outdated Show resolved Hide resolved
@kanpov kanpov requested a review from Eugeny July 9, 2024 08:18
@kanpov
Copy link
Contributor Author

kanpov commented Jul 9, 2024

I'm pretty sure I handled the reserved field correctly, but not completely sure

@Eugeny
Copy link
Owner

Eugeny commented Jul 9, 2024

I think you can go ahead and try it out :) looks mostly fine so far

@kanpov
Copy link
Contributor Author

kanpov commented Jul 9, 2024

I think you can go ahead and try it out :) looks mostly fine so far

I'll try by including the crate by path, hope it works

@kanpov
Copy link
Contributor Author

kanpov commented Jul 9, 2024

Update: it doesn't work :( RequestDenied, gonna try to investigate. Definitely my fault since ssh -L forwards just fine

@Eugeny
Copy link
Owner

Eugeny commented Jul 9, 2024

streamlocal-forward is -R :)

@kanpov
Copy link
Contributor Author

kanpov commented Jul 9, 2024

Well -L also works for some reason

@kanpov
Copy link
Contributor Author

kanpov commented Jul 9, 2024

@Eugeny Makes no sense, where's the issue here?

Impl:

            push_packet!(enc.write, {
                enc.write.push(msg::GLOBAL_REQUEST);
                enc.write
                    .extend_ssh_string(b"[email protected]");
                enc.write.push(want_reply as u8);
                enc.write.extend_ssh_string(socket_path.as_bytes());
            });

Spec:

	byte		SSH2_MSG_GLOBAL_REQUEST
	string		"[email protected]"
	boolean		TRUE
	string		socket path

@kanpov
Copy link
Contributor Author

kanpov commented Jul 9, 2024

@Eugeny While testing this, I also tested tcp/ip forwarding with handle.tcpip_forward and even that didn't work, even though a port was returned that port didn't have any connectivity.

@kanpov
Copy link
Contributor Author

kanpov commented Jul 9, 2024

So it seems forwarding generally is completely broken.

@kanpov
Copy link
Contributor Author

kanpov commented Jul 12, 2024

@Eugeny Considering forwarding is broken, I'm pretty sure making it work is not as simple as just sending global messages to enable/cancel forwarding over SSH.

My brain is too smooth to understand openssh's code, but I suppose what they're doing is not just making these global requests (which russh is capable of doing correctly, with some caveats), but also actually binding to these sockets itself and proxying all traffic over.

Unfortunately, there is little reference aside from openssh C code (which I'll still try to dig into sometime) about how exactly this is done, since openssh is one of the only implementations of ssh that actually support forwarding.

@kanpov
Copy link
Contributor Author

kanpov commented Aug 4, 2024

@Eugeny Have you tested whether this actually works after your fixes? Sorry for abandoning the PR

@Eugeny
Copy link
Owner

Eugeny commented Aug 4, 2024

Fixed a few issues and now it works:

  • Removed duplicate implementation for non-standard client-to-server forwarding channels
  • Added the missing @openssh.com in channel types and responses
  • The server doesn't return a socket path in the response

@Eugeny
Copy link
Owner

Eugeny commented Aug 4, 2024

No worries! It works with simple client & server examples - but I just tested it briefly

@Eugeny Eugeny marked this pull request as ready for review August 20, 2024 19:59
@Eugeny Eugeny merged commit 67a6ba8 into Eugeny:main Aug 20, 2024
3 of 4 checks passed
EpicEric pushed a commit to EpicEric/russh that referenced this pull request Nov 19, 2024
* docs: update pdocs

docs: fixed link

* fix: fmt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants