Skip to content
This repository has been archived by the owner on Feb 4, 2020. It is now read-only.

Avoid overwriting modified DNS upon restore #199

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

elias-pap
Copy link

In openvpn_disconnect(), "modify_dns revert_to_backup" is called to
restore DNS to the value backed-up by openvpn_connect(). However,
DNS might change after the backup and before a --disconnect is issued.
In that case, --disconnect causes a needless overwrite of DNS with an
old value, that might as well break the internet connection.

Use case:

  • Client connects to protonvpn.
  • Client changes access point. Internet connection breaks.
  • Client issues a --disconnect to restore internet connection.
  • Internet connection remains broken due to DNS overwrite.
    New --connect attempts will fail.

In openvpn_disconnect(), "modify_dns revert_to_backup" is called to
restore DNS to the value backed-up by openvpn_connect(). However,
DNS might change after the backup and before a --disconnect is issued.
In that case, --disconnect causes a needless overwrite of DNS with an
old value, that might as well break the internet connection.

Use case:

  * Client connects to protonvpn.
  * Client changes access point. Internet connection breaks.
  * Client issues a --disconnect to restore internet connection.
  * Internet connection remains broken due to DNS overwrite.
    New --connect attempts will fail.
@Tamaranch
Copy link
Contributor

There is a mistake at line 370 (Custom DNS). To avoid this and make the code more readable, both echo -e… should be replaced by cp "/etc/resolv.conf" "$(get_protonvpn_cli_home)/.resolv.conf.protonvpn_applied_backup"

@elias-pap
Copy link
Author

There is a mistake at line 370 (Custom DNS).

Thanks for pointing that out.

To avoid this and make the code more readable, both echo -e… should be replaced by cp "/etc/resolv.conf" "$(get_protonvpn_cli_home)/.resolv.conf.protonvpn_applied_backup"

Not really, because someone might change the resolv.conf in between (although it is quite unlikely to happen). I guess tee is the best/cleaner solution.

@Tamaranch
Copy link
Contributor

Yes, you're right! :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants