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.

Sign up for GitHub

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

Jump to bottom

Revert /etc/resolv.conf on app exit #3817

Closed
jamilbk opened this issue Feb 29, 2024 · 7 comments · Fixed by #4148
Closed

Revert /etc/resolv.conf on app exit #3817

jamilbk opened this issue Feb 29, 2024 · 7 comments · Fixed by #4148
Assignees
@ReactorScram
Labels
area/linux_client Linux client kind/UX Changes related specifically to overall user experience and product quality
Milestone
1.0 GA

Comments

@jamilbk
Copy link
Member

jamilbk commented Feb 29, 2024 •
edited
Loading

This will handle the happy path case in the headless linux client where a user has rebooted, restarted the client, or restarted the Docker container restarted via systemd.
@jamilbk jamilbk added the area/linux_client Linux client label Feb 29, 2024
@jamilbk jamilbk added this to the 1.0 GA milestone Feb 29, 2024
@jamilbk
Copy link
Member Author

jamilbk commented Feb 29, 2024

It would be nice if there was some way to simply mask /etc/resolv.conf with our own while the process is alive. I could have sworn Linux had some fancy linking that could do this.

@jamilbk jamilbk added the kind/UX Changes related specifically to overall user experience and product quality label Feb 29, 2024
@jamilbk jamilbk mentioned this issue Feb 29, 2024
Merged
@ReactorScram
Copy link
Collaborator

ReactorScram commented Feb 29, 2024 •
edited
Loading

The closest I can think of is using a mount like https://github.com/xetdata/nfsserve to overlay it, assuming Linux has an option to auto-unmount an NFS share if the server quits responding. This would also revert it on reboot.

We could even do a bind mount without NFS and that would at least revert it on reboot and simplify the manual revert command from mv /etc/resolv.conf.before-firezone /etc/resolv.conf to umount /etc/resolv.conf

I did some searches and asked ChatGPT but nothing obvious came up. Everyone says basically, trap this signal in your Bash script. If we have to run a Bash sidecar we may as well do something else entirely

This was referenced Mar 1, 2024
Merged
Merged
github-merge-queue bot pushed a commit that referenced this issue Mar 1, 2024
@ReactorScram
refactor(linux): make a place for reverting /etc/resolv.conf (#3822)
789d216 
Makes progress towards #3817

---------

Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
github-merge-queue bot pushed a commit that referenced this issue Mar 4, 2024
@ReactorScram
fix: use atomicwrites to back up /etc/resolv.conf more robustly (#…
c6c4624 
…3828)

Part of a small yak shave for #3817

The `atomicwrites` lib uses the atomic rename trick and does correct
fsyncs for us, so if we lose power while rewriting or reverting
`/etc/resolv.conf`, it should always be in a recoverable state. (Unless
the hard drive lies about fsync, but then it's beyond our control.)

---------

Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
@jamilbk
Copy link
Member Author

jamilbk commented Mar 13, 2024

We could instead modify the file in-place by adding our configuration to a sentinel stanza:

# BEGIN Firezone DNS configuration
nameserver 100.100.111.1
# END Firezone DNS configuration

@ReactorScram
Copy link
Collaborator

I think we'd have to prefix all other lines with # ? Or will later servers in resolv.conf be ignored as long as our responds fast enough?

@jamilbk
Copy link
Member Author

jamilbk commented Mar 13, 2024

We insert our stanza in the top, beginning line 1. Order matters here.

@ReactorScram
Copy link
Collaborator

Okay. But that means if we take too long, network clients will move on to other DNS servers below us, right?

@jamilbk
Copy link
Member Author

jamilbk commented Mar 13, 2024

network clients will move on to other DNS servers below us, right?

Correct, which might actually be a feature not a bug?

ReactorScram added a commit that referenced this issue Mar 14, 2024
@ReactorScram
feat(linux): revert /etc/resolv.conf on exit. Closes #3817
4b6f6c6
@ReactorScram ReactorScram mentioned this issue Mar 14, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Mar 18, 2024
@ReactorScram
chore(linux): revert /etc/resolv.conf on exit if we changed it to c…
a10d76c 
…ontrol DNS (#4148)

This isn't really user-facing, so I marked it down from `feat` to
`chore`. Closes #3817

- If we exit gracefully, `/etc/resolv.conf` is reverted
- We always keep the `.before-firezone` backup in case we lose power and
the revert transaction is corrupted or rolled back
- We use a magic header to detect whether the last run was a crash or
not. If Firezone crashes and the user wants to modify their default DNS,
they need to delete that header so that Firezone won't accidentally
revert its backup and trash their change.
- All error variants for this module replaced with `anyhow::Error` since
they were never matched by callers.

I ran `cargo mutants` locally and it helped me validate the unit tests
and it picked up a `match` branch that I forgot to delete.

```[tasklist]
- [x] (Failed: Integration tests didn't like it) ~~Add the system default resolvers below Firezone's sentinels~~
- [x] `tracing::info` "Last run crashed" if we have to revert the file at startup
```

---------

Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ReactorScram ReactorScram

Labels
area/linux_client Linux client kind/UX Changes related specifically to overall user experience and product quality
Projects
None yet
Milestone
1.0 GA
Development

Successfully merging a pull request may close this issue.

chore(linux): revert /etc/resolv.conf on exit if we changed it to control DNS firezone/firezone
2 participants
@jamilbk @ReactorScram