search

opnsense / plugins

OPNsense plugin collection
https://opnsense.org/
BSD 2-Clause "Simplified" License
832 stars 617 forks source link

security/openconnect clobbers /etc/resolv.conf #3090

Closed syserr0r closed 1 year ago

syserr0r commented 2 years ago

Important notices Before you add a new report, we ask you kindly to acknowledge the following:

Describe the bug OpenConnect overwrites /etc/resolv.conf with DNS settings from server.

It looks like this is done by /usr/local/sbin/vpnc-script.

Some suggestions for changing this behaviour are here: https://serverfault.com/questions/331299/how-can-i-stop-openconnect-from-changing-etc-resolv-conf/900825#900825

To Reproduce Steps to reproduce the behavior:

  1. Set-up OpenConnect VPN in 'Fortinet Fortigate' mode
  2. Connect
  3. Examine /etc/resolv.conf

Expected behavior New entries are not added to /etc/resolv.conf unless DNS settings specified in System > Settings > General > Networking

Screenshots N/A

Relevant log files N/A

Additional context Add any other context about the problem here.

Environment OPNsense 22.7.1 (amd64, OpenSSL).

syserr0r commented 2 years ago

I have somewhat worked around this by setting up a duplicate monit script (as described in #3091) but using the following start command: /usr/local/etc/rc.resolv_conf_generate - and leaving the stop command empty.

Incidentally the monit configuration doesn't seem to allow for commands without an argument (hence the - argument).

Edit: This doesn't work reliably so I have a cronjob set-up to run that script every 5 minutes. Messy but we need the OCVPN sadly.

syserr0r commented 1 year ago

This work-around is now broken in the latest release (23.1) as /usr/local/etc/rc.resolv_conf_generate no longer exists.

The new work-around is to run the following command instead: /usr/local/bin/php -r "require_once('config.inc'); require_once('util.inc'); require_once('system.inc'); require_once('interfaces.inc'); system_resolvconf_generate(true);"

This appears to be getting ever more fragile which is unfortunate

fichtner commented 1 year ago

@syserr0r it's been superseded by configctl dns reload -- if you want you can add a PR for core to add a description so you can use it from cron. :)

syserr0r commented 1 year ago

Thanks @fichtner, I wasn't aware of this -- I've made https://github.com/opnsense/core/pull/6290 I already run the above command from cron via a custom action on a regular interval.

The main impact is on breaking our VPN users as the local LDAP server cannot be accessed when the openconnect nameservers replace the system ones. I could put the LDAP server in by IP but then I'd have to relax the TLS certificate verification...

fichtner commented 1 year ago

@syserr0r can you try c77dd94 instead?

# opnsense-patch -c plugins c77dd94

Cheers, Franco

syserr0r commented 1 year ago

Sadly it doesn't work, /etc/resolv.conf is still overwritten

root@firewall:/home/tony # opnsense-patch -c plugins c77dd94
Fetched c77dd94 via https://github.com/opnsense/plugins
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|From c77dd94ad40a9ecefcf46dd1017de0712b89e5e8 Mon Sep 17 00:00:00 2001
|From: Franco Fichtner <franco@opnsense.org>
|Date: Wed, 1 Feb 2023 10:36:39 +0100
|Subject: [PATCH] security/openconnect: ignore DNS information #3090
|
|---
| .../src/opnsense/scripts/OPNsense/Openconnect/vpnc.sh     | 8 ++++++++
| .../service/templates/OPNsense/Openconnect/openconnect    | 3 ++-
| 2 files changed, 10 insertions(+), 1 deletion(-)
| create mode 100755 security/openconnect/src/opnsense/scripts/OPNsense/Openconnect/vpnc.sh
|
|diff --git a/security/openconnect/src/opnsense/scripts/OPNsense/Openconnect/vpnc.sh b/security/openconnect/src/opnsense/scripts/OPNsense/Openconnect/vpnc.sh
|new file mode 100755
|index 0000000000..1257be1029
|--- /dev/null
|+++ b/security/openconnect/src/opnsense/scripts/OPNsense/Openconnect/vpnc.sh
--------------------------
(Creating file opnsense/scripts/OPNsense/Openconnect/vpnc.sh...)
Patching file opnsense/scripts/OPNsense/Openconnect/vpnc.sh using Plan A...
Empty context always matches.
Hunk #1 succeeded at 1.
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git a/security/openconnect/src/opnsense/service/templates/OPNsense/Openconnect/openconnect b/security/openconnect/src/opnsense/service/templates/OPNsense/Openconnect/openconnect
|index 5b8dd90376..25975f7f93 100644
|--- a/security/openconnect/src/opnsense/service/templates/OPNsense/Openconnect/openconnect
|+++ b/security/openconnect/src/opnsense/service/templates/OPNsense/Openconnect/openconnect
--------------------------
Patching file opnsense/service/templates/OPNsense/Openconnect/openconnect using Plan A...
Hunk #1 succeeded at 1.
done
All patches have been applied successfully.  Have a nice day.

current nameservers (bad - provided by openconnect)

root@firewall:/home/tony # cat /etc/resolv.conf
# Generated by resolvconf
nameserver 100.65.0.2

Rewrite nameservers, what they should look like

root@firewall:/home/tony # configctl dns reload
Generating /etc/resolv.conf...done.
Generating /etc/hosts...done.
root@firewall:/home/tony # cat /etc/resolv.conf
domain site.company.com
nameserver 127.0.0.1
nameserver <local IPv4 resolver>
nameserver <local IPv6 resolver>
search site.company.com

Restart openconnect and show nameserver confing (still bad - from openconnect)

root@firewall:/home/tony # /usr/local/etc/rc.d/opnsense-openconnect stop
stopping openconnect
tun30000
root@firewall:/home/tony # /usr/local/etc/rc.d/opnsense-openconnect start
starting openconnect
ifconfig: interface tun30000 does not exist
ocvpn0
root@firewall:/home/tony # cat /etc/resolv.conf
# Generated by resolvconf
nameserver 100.65.0.2
fichtner commented 1 year ago

A bit strange, can you check in ps what command line arguments openconnect gets?

fichtner commented 1 year ago

Oh you need to reconfigure from the gui, not stop/start

syserr0r commented 1 year ago

Oh you need to reconfigure from the gui, not stop/start

Perfect, it looks like it is working now (including restarts from the CLI), thank-you 👍

fichtner commented 1 year ago

Hehe, yay. I hope @mimugmail won’t mind ;)

syserr0r commented 1 year ago

Is it worth making this a GUI toggle as some people could be relying on old behaviour (the openconnect DNS servers)?

That said it should only affect the local system... unless unbound/dnsmasq is set to use the system resolvers...

fichtner commented 1 year ago

Well, I think for the firewall this should be default behaviour. Probably not too many users for this who cannot work around this from the server end?

I’d try to ship this as is and if it needs a toggle we can add it later via the reporter’s ticket.