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

Add DNS to TUN, in VPN-Client #1381

Merged
merged 19 commits into from
Dec 14, 2022
Merged

Add DNS to TUN, in VPN-Client #1381

merged 19 commits into from
Dec 14, 2022

Conversation

mrpalide
Copy link
Contributor

@mrpalide mrpalide commented Oct 5, 2022

Did you run make format && make check?

Fixes #993

Changes (Tasks):

  • set -dns value on config and read it when vpn-client run.
  • modify app put to get -dns value from UI vpn-client setting dialogue
  • make change on UI, on skywire vpn ui settings
  • make changes on UI, on skywire manager vpn-client setting
    needs to add dns as another parameter in request, like this:
    {"pk":"03c46d47ecf7a0041eff843407312e0c8fad1b140037bbadbec7f38c6547a6dade","passcode":"","dns":"1.1.1.1"}
  • add dns set to tun process on linux
  • add dns set to tun process on darwin
  • add dns set to tun process on windows
  • setup skywire dns (maybe SKYDNS)
  • put skywire dns value on skywire-utilities skyenv
  • put skywire dns value on conf.skywire.dev and conf.skywire.skycoin.com
  • get dns value during generate config

How to test this PR:
Generate Config

  • just build make build, and generate config skywire-cli config gen -t
  • check vpn-client arguments in config file or in VPN UI -> Settings -> Custom DNS Server

Test DNS

  • remove dns arguments from vpn-client in config file, should be like:
    {
      "name": "vpn-client",
      "args": [
      	"-srv",
      	"03c46d47ecf7a0041eff843407312e0c8fad1b140037bbadbec7f38c6547a6dade"
      ],
      "auto_start": false,
      "port": 43
    },
    
  • start vpn-client
  • check dns of TUN (in linux and windows) or Wi-Fi (on darwin)
    • linux: nmcli dev show tun0
    • windows: netsh interface ipv4 show config tun0
    • darwin: networksetup -getdnsservers Wi-Fi or cat /etc/resolv.conf
  • check dns by online tools like DNS leak test
  • stop vpn-client, add DNS ip to VPN UI -> Settings -> Custom DNS Server value like 1.1.1.1, after that your config should be like this:
     {
      "name": "vpn-client",
      "args": [
      	"-srv",
      	"03c46d47ecf7a0041eff843407312e0c8fad1b140037bbadbec7f38c6547a6dade",
      	"-dns",
      	"1.1.1.1"
      ],
      "auto_start": false,
      "port": 43
    },
    
  • start vpn again
  • check dns of TUN (in linux and windows) or Wi-Fi (on darwin) again
  • check dns by online tools again
  • stop vpn
  • check dns of Wi-Fi (on darwin) again | In windows and linux TUN driver will delete, so nothing to be check be there

Screencasts:

dns-test-final.mov

@0pcom 0pcom added this to the Milestone 8 milestone Dec 13, 2022
@0pcom
Copy link
Collaborator

0pcom commented Dec 13, 2022

just fix the merge conflicts and we can merge this

@0pcom 0pcom self-requested a review December 14, 2022 19:57
@@ -29,6 +29,7 @@ var (
localSKStr = flag.String("sk", "", "Local SecKey")
passcode = flag.String("passcode", "", "Passcode to authenticate connection")
killswitch = flag.Bool("killswitch", false, "If set, the Internet won't be restored during reconnection attempts")
dnsAddr = flag.String("dns", "", "address of DNS want set to tun")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will come back and fix the flag text. Not sure what the best wording should be.

@@ -610,6 +611,36 @@ func (v *Visor) SetAppPK(appName string, pk cipher.PubKey) error {
return nil
}

// SetAppDNS implements API.
func (v *Visor) SetAppDNS(appName string, dnsAddr string) error {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, we are setting the DNS per the app here? I suppose that is useful but it makes me think that the DNS is not settable for the visor process itself.

Copy link
Collaborator

@0pcom 0pcom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@0pcom 0pcom merged commit a3d6630 into skycoin:develop Dec 14, 2022
@mrpalide mrpalide deleted the feature/set-dns-to-tun branch January 8, 2023 16:36
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