Skip to content

Fix DnsProxy#618

Merged
brianmay merged 1 commit intosshuttle:masterfrom
patrislav1:bugfix-dnsproxy
Mar 20, 2021
Merged

Fix DnsProxy#618
brianmay merged 1 commit intosshuttle:masterfrom
patrislav1:bugfix-dnsproxy

Conversation

@patrislav1
Copy link
Contributor

@patrislav1 patrislav1 commented Mar 18, 2021

When using sshuttle with DNS forwarding, the server part calls setsockopt with a string value for TTL and fails (see below). In my case the DnsProxy was initialized with a string value "63" as ttl argument.
This patch fixes it by casting ttl to int type.

sshuttle[4549]: c : DNS request from ('192.168.111.9', 33380): 35 bytes
sshuttle[4549]: c : DNS request from ('192.168.111.9', 58082): 35 bytes
sshuttle[4549]:  s: setsockopt: 63
sshuttle[4549]: Traceback (most recent call last):
sshuttle[4549]:   File "<string>", line 1, in <module>
sshuttle[4549]:   File "assembler.py", line 43, in <module>
sshuttle[4549]:   File "sshuttle.server", line 398, in main
sshuttle[4549]:   File "sshuttle.ssnet", line 616, in runonce
sshuttle[4549]:   File "sshuttle.ssnet", line 504, in callback
sshuttle[4549]:   File "sshuttle.ssnet", line 492, in handle
sshuttle[4549]:   File "sshuttle.ssnet", line 411, in got_packet
sshuttle[4549]:   File "sshuttle.server", line 354, in dns_req
sshuttle[4549]:   File "sshuttle.server", line 172, in __init__
sshuttle[4549]:   File "sshuttle.server", line 196, in try_send
sshuttle[4549]: TypeError: a bytes-like object is required, not 'str'

@patrislav1 patrislav1 mentioned this pull request Mar 19, 2021
Closed
@brianmay
Copy link
Member

brianmay commented Mar 20, 2021

Hmmm. I think this would come up with an exception error if the user enters a non-integer value.

I would rather see a solution that updates the argparser to take in integer instead of a string.

This code would need to be changed: https://github.com/sshuttle/sshuttle/blob/master/sshuttle/options.py#L390-L397

The very first example here shows how to take an integer. I think you need to add type=int and change the value of default= to be an integer. And then check everything still works.

@patrislav1
Copy link
Contributor Author

patrislav1 commented Mar 20, 2021

Adapted the patch as suggested. In some other place, it has to be converted to str again, when FirewallClient builds an argv list. Bottom line I still agree it is the better approach.

@brianmay brianmay merged commit bafc0a7 into sshuttle:master Mar 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@patrislav1 @brianmay