Skip to content

fix: require -r/--remote#502

Merged
brianmay merged 3 commits intosshuttle:masterfrom
joshuarli:ref/require-remote
Aug 11, 2020
Merged

fix: require -r/--remote#502
brianmay merged 3 commits intosshuttle:masterfrom
joshuarli:ref/require-remote

Conversation

@joshuarli
Copy link
Contributor

@joshuarli joshuarli commented Aug 10, 2020 •
edited
Loading

Fixes #455.

Before, as the issue describes:

$ sshuttle -v --ssh-cmd 'ssh example.com' 0/0
...
    if "@" in host:
TypeError: argument of type 'NoneType' is not iterable

This manifests much more horribly in versions before Python 2 support was removed btw. Just some fun trivia. Example sshuttle==0.78.5 with Python 3 interpreter:

sshuttle -v --ssh-cmd 'ssh -i ~/.ssh/example (example ip address)' 192.168.208.0/20 172.16.0.16/28 10.1.128.0/19 10.1.64.0/18 10.1.0.0/18
Starting sshuttle proxy.
-- snip --
Starting client with Python version 3.8.3
c : connecting to server...
assembler.py:3: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
Starting server with Python version 3.8.3
-- snip --
c : Accept TCP: 192.168.42.65:56955 -> 192.168.208.10:22.
c : Accept TCP: 192.168.42.65:56956 -> 192.168.208.10:22.
c : Accept TCP: 192.168.42.65:56957 -> 192.168.208.10:22.
c : Accept TCP: 192.168.42.65:56958 -> 192.168.208.10:22.
c : Accept TCP: 192.168.42.65:56959 -> 192.168.208.10:22.
c : Accept TCP: 192.168.42.65:56960 -> 192.168.208.10:22.
...

After:

$ sshuttle -v --ssh-cmd 'ssh example.com' 0/0
usage: sshuttle [-l [ip:]port] [-r [user@]sshserver[:port]] <subnets...>
sshuttle: error: the following arguments are required: -r/--remote

@joshuarli joshuarli mentioned this pull request Aug 10, 2020
Closed
@joshuarli
Copy link
Contributor Author

joshuarli commented Aug 10, 2020

Oh hold on, don't merge this yet.

@joshuarli
Copy link
Contributor Author

joshuarli commented Aug 10, 2020 •
edited
Loading

Ok, so we can't make it required at the argparse level because sshuttle calls out to itself in FirewallClient:

        argvbase = ([sys.executable, sys.argv[0]] +
                    ['-v'] * (helpers.verbose or 0) +
                    ['--method', method_name] +
                    ['--firewall'])

@joshuarli
Copy link
Contributor Author

joshuarli commented Aug 10, 2020

After:

Starting sshuttle proxy.
firewall manager: Starting firewall with Python version 3.8.3
firewall manager: ready method name pf.
IPv6 enabled: True
UDP enabled: False
DNS enabled: False
User enabled: False
TCP redirector listening on ('::1', 12300, 0, 0).
TCP redirector listening on ('127.0.0.1', 12300).
fatal: You must specify -r/--remote.

And now it works if you actually specify the argument.

I could move this fatal up the call stack a bit.

@joshuarli
Copy link
Contributor Author

joshuarli commented Aug 10, 2020

Ok, now it just fails outright with fatal: You must specify -r/--remote..

@brianmay Ready for your review.

@brianmay brianmay merged commit 45f8cce into sshuttle:master Aug 11, 2020
@brianmay
Copy link
Member

brianmay commented Aug 11, 2020

Thanks!

@joshuarli joshuarli deleted the ref/require-remote branch August 11, 2020 21:49
skuhl added a commit to skuhl/sshuttle that referenced this pull request Oct 19, 2020
@skuhl
Allow no remote to work.
574ed8e 
Pull request sshuttle#502 made -r/--remote required. However, the
documentation still indicates that using no remote is a valid way to
test sshuttle (see Examples section of man page). I think this mode
might be useful for testing performance local without ssh, local with
ssh, and remote with ssh.

This patch adds a warning when -r/--remote is missing but restores the
previous behavior.
@skuhl skuhl mentioned this pull request Oct 19, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brianmay brianmay brianmay approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

I keep getting the following error on python

2 participants

@joshuarli @brianmay