From 90864fdb297ae943a3208eb07c30014932a45d03 Mon Sep 17 00:00:00 2001 From: Andy Lemin Date: Thu, 3 Sep 2026 21:22:54 +1000 Subject: [PATCH 1/4] unbound: name events without strmodulevent With LOGGING on, every call raised OverflowError from strmodulevent: its binding rejects a value outside 'enum module_ev', and the value pythonmod passes is evidently outside it. A log line must not be able to fail, so the event is named against the MODULE_EVENT_* constants pythonmod injects, and an unrecognised value is printed as itself rather than converted. pythonmod: operate, id: 1, MODULE_EVENT_PASS pythonmod: operate, id: 1, event 7 --- client-unbound/pfui_unbound.py | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/client-unbound/pfui_unbound.py b/client-unbound/pfui_unbound.py index eb582ba..014735c 100644 --- a/client-unbound/pfui_unbound.py +++ b/client-unbound/pfui_unbound.py @@ -562,6 +562,28 @@ def inform_super(id, qstate, superqstate, qdata): return True +# pythonmod injects these into this module's globals. strmodulevent() is not +# used to name an event: its binding rejects anything outside 'enum module_ev' +# with an OverflowError, and a log line must not be able to fail. +EVENT_NAMES = ( + "MODULE_EVENT_NEW", + "MODULE_EVENT_PASS", + "MODULE_EVENT_REPLY", + "MODULE_EVENT_NOREPLY", + "MODULE_EVENT_CAPSFAIL", + "MODULE_EVENT_MODDONE", + "MODULE_EVENT_ERROR", +) + + +def event_name(event): + """Name for a pythonmod event value, or the value itself if unrecognised.""" + for name in EVENT_NAMES: + if globals().get(name) == event: + return name + return f"event {event!r}" + + def describe_exception(exc): """One-line type, message and call site for an exception. @@ -604,11 +626,7 @@ def operate(id, event, qstate, qdata): def _operate(id, event, qstate, qdata): if pfui_cfg["LOGGING"]: - log_info( - "pythonmod: operate, id: {}, event {}".format( - str(id), str(strmodulevent(event)) - ) - ) + log_info(f"pythonmod: operate, id: {id}, {event_name(event)}") if event == MODULE_EVENT_MODDONE: if pfui_cfg["LOGGING"]: From d42cd04f097d5b4eabc45a24633852357a610ce3 Mon Sep 17 00:00:00 2001 From: Andy Lemin Date: Thu, 3 Sep 2026 21:58:25 +1000 Subject: [PATCH 2/4] unbound: stop module faults from failing DNS queries Reviewing the callback and transmit paths turned up faults that each end in a served query being lost, all reachable from a configuration an operator can write. Every one gets a test that fails without its fix. A FIREWALLS key present but empty parses as None, which setdefault cannot replace, so transmit_all iterated None and raised on every answer carrying an address. The shipped config ships commented-out entries in exactly that shape. It is normalised at load, along with a check that it is a list. inplace_cache_callback was the one callback with no guard and no return value. An exception there discards the cache hit, and the callback's result is read as a boolean, so returning nothing leaves an error pending for the next query to fail on. It now returns True and logs what it caught. operate's handler set MODULE_WAIT_MODULE whatever the event, which at MODDONE tells the mesh to advance to the next module and re-enter one that has already run. A fault there now finishes the query. The circuit breaker could not open against a firewall that accepts and never answers: a cache report succeeds at the socket, and recording that a success cleared the failures the blocking path had counted, so with any cache hit between two resolutions the count never reached the threshold and every query kept paying SOCKET_TIMEOUT. A non-blocking send now records nothing. logger's guard against a reply with no records sat five lines after the dereference it guarded, and the MODDONE call site tested qinfo where it meant rep. Every numeric config value is coerced once at load, and PORT is checked there. A quoted or emptied number previously reached a socket call as a string or None and raised there, where the failure reads as a resolver fault rather than the configuration error it is. UDP no longer waits for an acknowledgement a failed send cannot produce, treats ACKUPDATE as proof of delivery rather than retransmitting to a firewall that has already updated PF, and skips the wait entirely for a cache report, which is fire-and-forget by design. --- client-unbound/pfui_unbound.py | 144 ++++++++++++++------ client-unbound/tests/test_unbound_module.py | 122 +++++++++++++++++ 2 files changed, 223 insertions(+), 43 deletions(-) diff --git a/client-unbound/pfui_unbound.py b/client-unbound/pfui_unbound.py index 014735c..2e7bd7f 100644 --- a/client-unbound/pfui_unbound.py +++ b/client-unbound/pfui_unbound.py @@ -76,6 +76,9 @@ def logger(qstate): r = qstate.return_msg.rep q = qstate.return_msg.qinfo + if not r or not q: + log_info(f"Query: {qstate.qinfo.qname_str} carried no reply to log") + return log_info("-" * 100) log_info( f"Query: {qstate.qinfo.qname_str}, " @@ -194,27 +197,34 @@ def read_rr(rep=None, qname_str="", from_cache=False): return False -def udp_transmit(soc, data, ip, port, retry=1): +def udp_transmit(soc, data, ip, port, retry=1, wait_for_ack=True): + """Send one datagram, optionally waiting for the firewall to acknowledge it. + + Returns the acknowledgement received, or None. ACKUPDATE counts as one: + the datagrams can arrive in either order, and taking only ACKDATA meant a + firewall that had already updated PF was retransmitted to and then counted + a failure. + """ tries = 0 - msg = None # retry may be 0, and the summary below reads this + msg = None while tries < retry: + tries += 1 try: log_info(f"PFUIDNS: UDP Transmitting {len(data)} bytes") soc.sendto(data, (ip, port)) - except TIMEOUT: - log_err( - f"PFUIDNS: UDP socket timeout {ip}:{port}" - ) except Exception as e: - log_err(f"PFUIDNS: UDP socket exception {ip}:{port}, '{e}'") - msg = udp_receive(soc=soc, rcvbuf=40, retry=1) # Wait pfui_firewall ack data - if msg == b"ACKDATA": # 40 - log_info(f"PFUIDNS: Received ACKDATA (transmit success): {msg}") + # No acknowledgement can follow a send that did not leave, so this + # does not pay the ACK timeout before retrying + log_err(f"PFUIDNS: UDP send failed {ip}:{port}, '{e}'") + continue + if not wait_for_ack: + return None + msg = udp_receive(soc=soc, rcvbuf=64, retry=1) + if msg in (b"ACKDATA", b"ACKUPDATE"): + log_info(f"PFUIDNS: Received {msg!r} (transmit success)") return msg - else: - log_info(f"PFUIDNS: Received message not ACKDATA: {msg}") - tries += 1 - log_info(f"PFUIDNS: timeout udp_transmit: buff {msg}") + log_info(f"PFUIDNS: Received message not an acknowledgement: {msg!r}") + log_info(f"PFUIDNS: no acknowledgement from {ip}:{port}, last was {msg!r}") def udp_receive(soc, rcvbuf=1400, retry=1): @@ -241,19 +251,21 @@ def udp_transmit_close(data, ip, port, blocking): # 3s each a down firewall blocked the resolver for ~2 minutes per query soc.settimeout(float(pfui_cfg["UDP_ACK_TIMEOUT"])) - # transmit pf firewall data - reply = udp_transmit(soc, data, ip, port, int(pfui_cfg["UDP_RETRY"])) - breaker_record(f"{ip}:{port}", ok=(reply == b"ACKDATA")) - - # wait for pf firewall update - if blocking: # Wait for secondary ACKUPDATE - msg = udp_receive(soc=soc, rcvbuf=42, retry=1) - if msg == b"ACKUPDATE": - log_info( - "PFUIDNS: Recv pfui_firewall Update ACK" - ) - else: - log_info(f"PFUIDNS: Unexpected msg: {msg}") + # A cache report is fire-and-forget, so it waits for nothing: holding the + # answer for an acknowledgement is what BLOCKING asks for, and this is not it + reply = udp_transmit( + soc, data, ip, port, pfui_cfg["UDP_RETRY"], wait_for_ack=blocking + ) + if blocking: + confirmed = reply == b"ACKUPDATE" + if not confirmed and reply == b"ACKDATA": + # ACKDATA only says the message decoded; the tables are updated by + # the time ACKUPDATE follows + msg = udp_receive(soc=soc, rcvbuf=64, retry=1) + confirmed = msg == b"ACKUPDATE" + if not confirmed: + log_err(f"PFUIDNS: {ip}:{port} did not confirm the update: {msg!r}") + breaker_record(f"{ip}:{port}", ok=confirmed) # close sender udp socket soc.close() @@ -358,9 +370,9 @@ def stream_transmit_close(data, family, address, target, blocking): log_err( f"PFUIDNS: {target} did not confirm the update: {reply!r}" ) - elif sent: - # Non-blocking: delivery is the only thing observable from here - breaker_record(target, ok=True) + # A non-blocking send records nothing: the firewall has not answered + # yet, and counting it a success cleared the failures the blocking path + # was accumulating, so the breaker could never open except TIMEOUT: breaker_record(target, ok=False) log_err( @@ -490,19 +502,26 @@ def transmit_all(pfui_dict, blocking=True): def inplace_cache_callback( qinfo, qstate, rep, rcode, edns, opt_list_out, region, **kwargs ): - """pythonmod: Inplace callback function for cache responses.""" - if pfui_cfg["LOGGING"] and pfui_cfg["LOG_LEVEL"] == "DEBUG": - log_info("pythonmod: cache_callback called - answering from cache.") + """pythonmod: Inplace callback function for cache responses. - if pfui_cfg["LOGGING"] and pfui_cfg["LOG_LEVEL"] == "DEBUG": - log_info( - f"Cache data - qinfo: {qinfo}, qstate: {qstate}, rep: {rep}, rcode: {rcode}, edns: {edns}, opt_list_out: {opt_list_out}, region: {region}" - ) + Returns True, and never raises: an exception here discards the cache hit, + and the callback's return value is read as a boolean, so returning nothing + leaves an error pending for the next query to fail on. + """ + try: + if pfui_cfg["LOGGING"] and pfui_cfg["LOG_LEVEL"] == "DEBUG": + log_info("pythonmod: cache_callback called - answering from cache.") + log_info( + f"Cache data - qinfo: {qinfo}, qstate: {qstate}, rep: {rep}, rcode: {rcode}, edns: {edns}, opt_list_out: {opt_list_out}, region: {region}" + ) - if rep is not None: - pfui_msg = read_rr(rep, qinfo.qname_str, from_cache=True) - if pfui_msg: - transmit_all(pfui_msg, blocking=False) + if rep is not None: + pfui_msg = read_rr(rep, qinfo.qname_str, from_cache=True) + if pfui_msg: + transmit_all(pfui_msg, blocking=False) + except Exception as exc: + log_err(f"PFUIDNS: {describe_exception(exc)}") + return True def init(id, cfg): @@ -532,6 +551,10 @@ def init_standard(id, env): ) if not register_inplace_cb_reply_cache(inplace_cache_callback, env, id): + log_err( + "PFUIDNS: could not register the cache callback; cache hits will " + "whitelist nothing. Refusing to start." + ) return False return True @@ -617,7 +640,13 @@ def operate(id, event, qstate, qdata): except Exception as exc: log_err(f"PFUIDNS: {describe_exception(exc)}") try: - qstate.ext_state[id] = MODULE_WAIT_MODULE + # A finished query must not be handed back for more work: the mesh + # reads MODULE_WAIT_MODULE as "advance to the next module", which + # re-enters one that has already run + if event == MODULE_EVENT_MODDONE: + qstate.ext_state[id] = MODULE_FINISHED + else: + qstate.ext_state[id] = MODULE_WAIT_MODULE except Exception: pass return True @@ -636,7 +665,7 @@ def _operate(id, event, qstate, qdata): pfui_msg = None if qstate.return_msg: if pfui_cfg["LOGGING"] and pfui_cfg["LOG_LEVEL"] == "DEBUG": - if qstate.return_msg.qinfo: + if qstate.return_msg.rep and qstate.return_msg.qinfo: logger(qstate) if qstate.return_msg.rep: pfui_msg = read_rr(qstate.return_msg.rep, qstate.qinfo.qname_str) @@ -699,6 +728,28 @@ def load_config(location=CONFIG_LOCATION): cfg = safe_load(open(location)) or {} for key, value in CONFIG_DEFAULTS.items(): cfg.setdefault(key, value) + + # A key present but empty parses as None, which setdefault cannot replace + cfg["FIREWALLS"] = cfg["FIREWALLS"] or [] + if not isinstance(cfg["FIREWALLS"], list): + raise ValueError(f"FIREWALLS must be a list, not {cfg['FIREWALLS']!r}") + + # Coerced once here so no send path can be handed a string or a None: those + # raise from inside socket calls, where the failure is a resolver fault + # rather than the configuration error it really is + for key, cast in ( + ("SOCKET_TIMEOUT", float), + ("UDP_ACK_TIMEOUT", float), + ("BREAKER_COOLOFF", float), + ("UDP_RETRY", int), + ("BREAKER_FAILURES", int), + ("DEFAULT_PORT", int), + ): + try: + cfg[key] = cast(cfg[key]) + except (TypeError, ValueError): + raise ValueError(f"{key} must be a number, not {cfg[key]!r}") from None + cfg["SOCKET_PROTO"] = str(cfg["SOCKET_PROTO"]).strip().upper() if cfg["SOCKET_PROTO"] not in ("TCP", "UDP"): raise ValueError( @@ -716,6 +767,13 @@ def load_config(location=CONFIG_LOCATION): f"FIREWALLS[{index}] sets both SOCKET ({socket_path}) and HOST " f"({host}); one firewall is reached one way or the other" ) + if host and fw.get("PORT") is not None: + try: + int(fw["PORT"]) + except (TypeError, ValueError): + raise ValueError( + f"FIREWALLS[{index}] PORT must be a number, not {fw['PORT']!r}" + ) from None if socket_path and not str(socket_path).startswith("/"): raise ValueError( f"FIREWALLS[{index}] SOCKET must be an absolute path, " diff --git a/client-unbound/tests/test_unbound_module.py b/client-unbound/tests/test_unbound_module.py index bfec101..3a0af1f 100644 --- a/client-unbound/tests/test_unbound_module.py +++ b/client-unbound/tests/test_unbound_module.py @@ -730,3 +730,125 @@ class QState: qstate = QState() assert plugin.operate(0, 999, qstate, None) is True assert qstate.ext_state[0] == plugin.MODULE_ERROR + + +# Faults found reviewing the callback and transmit paths. Each of these failed +# before its fix, and each is reachable from a config an operator can write. + + +def test_a_commented_out_firewall_list_loads_as_a_list(plugin, tmp_path): + """FIREWALLS: with every entry commented out parses as None, which + setdefault cannot replace, and transmit_all then iterates None.""" + cfg = tmp_path / "pfui_unbound.yml" + cfg.write_text("LOGGING: False\nFIREWALLS:\n# - HOST: 10.0.0.1\n") + assert plugin.load_config(cfg)["FIREWALLS"] == [] + + +def test_transmit_all_survives_an_empty_firewall_list(plugin, tmp_path): + cfg = tmp_path / "pfui_unbound.yml" + cfg.write_text("LOGGING: False\nFIREWALLS:\n") + plugin.pfui_cfg = plugin.load_config(cfg) + plugin.transmit_all( + {"kind": "rr", "qname": "x.", "AF4": [{"ip": "8.8.8.8", "ttl": 60}], "AF6": []}, + False, + ) + + +@pytest.mark.parametrize( + "line", + ["SOCKET_TIMEOUT: '3'", "UDP_RETRY: '3'", "BREAKER_FAILURES: '3'", + "UDP_ACK_TIMEOUT: '0.5'"], +) +def test_quoted_numbers_are_coerced_at_load(plugin, tmp_path, line): + """A quoted number reached the socket calls as a string and raised there.""" + cfg = tmp_path / "pfui_unbound.yml" + cfg.write_text(f"FIREWALLS:\n - HOST: 10.0.0.1\n{line}\n") + key = line.split(":")[0] + assert isinstance(plugin.load_config(cfg)[key], (int, float)) + + +@pytest.mark.parametrize("value", ["1O001", "", "abc"]) +def test_an_unusable_port_is_refused_at_load(plugin, tmp_path, value): + """int(PORT) runs on the send path, where the failure looks like a resolver + fault rather than the configuration error it is.""" + cfg = tmp_path / "pfui_unbound.yml" + cfg.write_text(f"FIREWALLS:\n - HOST: 10.0.0.1\n PORT: '{value}'\n") + with pytest.raises(ValueError, match="PORT"): + plugin.load_config(cfg) + + +def test_an_emptied_numeric_key_is_refused_at_load(plugin, tmp_path): + cfg = tmp_path / "pfui_unbound.yml" + cfg.write_text("FIREWALLS:\n - HOST: 10.0.0.1\nBREAKER_FAILURES:\n") + with pytest.raises(ValueError, match="BREAKER_FAILURES"): + plugin.load_config(cfg) + + +def test_a_non_blocking_send_records_no_success(plugin): + """A firewall that accepts and never answers is a failure the blocking path + counts. A cache report to the same firewall succeeds at the socket, and + recording that a success cleared the count, so the breaker never opened + however long the firewall stayed silent.""" + listener = socket.socket() + listener.bind(("127.0.0.1", 0)) + listener.listen(1) + host, port = listener.getsockname() + plugin.pfui_cfg = dict(plugin.CONFIG_DEFAULTS, SOCKET_TIMEOUT=0.2, + BREAKER_FAILURES=3, BREAKER_COOLOFF=30) + plugin._breakers.clear() + recorded = [] + original = plugin.breaker_record + + def spy(target, ok): + recorded.append(ok) + original(target, ok) + + plugin.breaker_record = spy + try: + # Accepted but never answered, exactly what a saturated firewall does + plugin.tcp_transmit_close(b"x", host, port, blocking=False) + assert True not in recorded, f"a non-blocking send recorded {recorded}" + + for _ in range(3): + plugin.tcp_transmit_close(b"x", host, port, blocking=True) + assert plugin.breaker_open(f"{host}:{port}") + finally: + plugin.breaker_record = original + listener.close() + + +def test_the_cache_callback_reports_a_result_and_never_raises(plugin, monkeypatch): + """Its return value is read as a boolean, and a raise discards the cache hit.""" + monkeypatch.setattr(plugin, "read_rr", + lambda *a, **k: {"kind": "cache", "qname": "x.", + "AF4": [], "AF6": []}) + monkeypatch.setattr(plugin, "transmit_all", + lambda *a, **k: (_ for _ in ()).throw(RuntimeError("boom"))) + qinfo = type("Q", (), {"qname_str": "x."})() + assert plugin.inplace_cache_callback( + qinfo, None, object(), 0, None, None, None + ) is True + + +def test_a_fault_at_moddone_finishes_the_query(plugin, monkeypatch): + """MODULE_WAIT_MODULE tells the mesh to advance to the next module, which + re-enters one that has already run and loops the query.""" + monkeypatch.setattr(plugin, "read_rr", + lambda *a, **k: (_ for _ in ()).throw(RuntimeError("boom"))) + qstate = type("S", (), { + "return_msg": type("R", (), {"rep": object(), "qinfo": object()})(), + "qinfo": type("Q", (), {"qname_str": "x."})(), + "ext_state": {}, + })() + assert plugin.operate(0, plugin.MODULE_EVENT_MODDONE, qstate, None) is True + assert qstate.ext_state[0] == plugin.MODULE_FINISHED + + +def test_logger_tolerates_a_reply_without_records(plugin): + """The 'if r:' guard sat five lines after the dereference it guarded.""" + qstate = type("S", (), { + "return_msg": type("R", (), {"rep": None, "qinfo": object()})(), + "qinfo": type("Q", (), {"qname_str": "x.", "qtype_str": "A", "qtype": 1, + "qclass_str": "IN", "qclass": 1})(), + })() + plugin.logger(qstate) From ecc7f080f807ac9fbd31f09f06f6c2991dcf0918 Mon Sep 17 00:00:00 2001 From: Andy Lemin Date: Thu, 3 Sep 2026 22:02:28 +1000 Subject: [PATCH 3/4] unbound: read the whole acknowledgement, not one segment A single recv() took whatever the first segment carried, so an ACKUPDATE delivered in two pieces compared unequal, was logged as a refusal and was charged to the circuit breaker as a failure against a firewall that had just confirmed the update. The reply is read until the firewall closes. --- client-unbound/pfui_unbound.py | 10 ++++++- client-unbound/tests/test_unbound_module.py | 31 +++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/client-unbound/pfui_unbound.py b/client-unbound/pfui_unbound.py index 2e7bd7f..a663fb2 100644 --- a/client-unbound/pfui_unbound.py +++ b/client-unbound/pfui_unbound.py @@ -362,7 +362,15 @@ def stream_transmit_close(data, family, address, target, blocking): # keeps denying the traffic either way. try: if blocking and sent: # Nothing to acknowledge if the send failed - reply = conn.recv(36) # Wait for pfui_firewall to ACK + # Read until the firewall closes rather than taking one segment: a + # reply split in transit compared unequal to ACKUPDATE and was + # charged to the breaker as a refusal + reply = b"" + while len(reply) < 36: + chunk = conn.recv(36 - len(reply)) + if not chunk: + break + reply += chunk breaker_record(target, ok=(reply == b"ACKUPDATE")) if reply != b"ACKUPDATE": # The firewall replies with a reason when it refuses a message, diff --git a/client-unbound/tests/test_unbound_module.py b/client-unbound/tests/test_unbound_module.py index 3a0af1f..fb329f4 100644 --- a/client-unbound/tests/test_unbound_module.py +++ b/client-unbound/tests/test_unbound_module.py @@ -852,3 +852,34 @@ def test_logger_tolerates_a_reply_without_records(plugin): "qclass_str": "IN", "qclass": 1})(), })() plugin.logger(qstate) + + +def test_an_acknowledgement_split_in_transit_is_still_read(plugin): + """One recv() took whatever the first segment held, so a reply delivered in + two pieces compared unequal to ACKUPDATE and was charged as a refusal.""" + import threading + import time as _time + + listener = socket.socket() + listener.bind(("127.0.0.1", 0)) + listener.listen(1) + host, port = listener.getsockname() + + def serve(): + conn, _ = listener.accept() + conn.recv(1024) + conn.send(b"ACK") # deliberately split + _time.sleep(0.05) + conn.send(b"UPDATE") + conn.close() + + threading.Thread(target=serve, daemon=True).start() + plugin.pfui_cfg = dict(plugin.CONFIG_DEFAULTS, SOCKET_TIMEOUT=2, + BREAKER_FAILURES=3, BREAKER_COOLOFF=30) + plugin._breakers.clear() + try: + plugin.tcp_transmit_close(b"x", host, port, blocking=True) + # A confirmed update leaves no failure counted against the firewall + assert plugin._breakers[f"{host}:{port}"][0] == 0 + finally: + listener.close() From 8a4179591bd7965c0af119673256d1e1fb021303 Mon Sep 17 00:00:00 2001 From: Andy Lemin Date: Thu, 3 Sep 2026 22:05:12 +1000 Subject: [PATCH 4/4] unbound: make rebuilding the resolver an explicit choice Updating the PFUI module is the common case and needs no rebuild, but the prompt said the build was required, and the source-tree question came first, so a module-only upgrade was asked to decide about replacing /usr/src before it had said whether it was building at all. The build is now a choice that reports what is installed and defaults to the sensible answer: keep an Unbound that already has the Python module, build one when there is none or it cannot load a module. Skipping says what it is doing and warns when the installed resolver cannot run PFUI. The source-tree choice moved inside the build path, which is the only thing that needs sources. --- README.md | 3 +- install-client-unbound.sh | 86 ++++++++++++++++++++++++++------------- 2 files changed, 60 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index 10b04ec..10c9b9a 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,8 @@ adds latency). A fully recursive DNS query can take tens to hundreds of millisec Tested on OpenBSD from 7.0 (Unbound 1.16, Python 3.8) to 7.9 (Unbound 1.26, Python 3.13). **Re-running an installer upgrades the code and leaves your configuration -alone.** `/etc/pfui_firewall.yml`, `pfui_unbound.yml` and the resolver's own +alone**, and the resolver installer offers to skip rebuilding Unbound, which is +what a module-only upgrade wants. `/etc/pfui_firewall.yml`, `pfui_unbound.yml` and the resolver's own `pfui_unbound.conf` are kept as they are, with a timestamped backup taken and the shipped example named so you can diff it for keys added since. Only a first install, where no config exists yet, lays down the examples. diff --git a/install-client-unbound.sh b/install-client-unbound.sh index c91f78f..0fe0837 100755 --- a/install-client-unbound.sh +++ b/install-client-unbound.sh @@ -152,32 +152,6 @@ if [[ "$OS" = "OpenBSD" ]]; then fi echo - # Unbound is built with Makefile.bsd-wrapper taken from the system sources, - # so a usable /usr/src is a prerequisite for the build below - if [ -f /usr/src/usr.sbin/unbound/Makefile.bsd-wrapper ] \ - || [ -f /usr/src/usr.sbin/unbound.base/Makefile.bsd-wrapper ]; then - HAVE_SRC="found" - else - HAVE_SRC="NOT found" - fi - - REL=$(uname -r) - echo "OpenBSD system sources in /usr/src (Unbound's wrapper Makefile comes from there)" - echo " 1) keep the existing tree and carry on to the build [${HAVE_SRC}]" - echo " 2) replace it with the signed ${REL} release sources" - echo " 3) replace it with -current from the git mirror (unsigned)" - echo "Options 2 and 3 delete /usr/src/* first. /usr/ports is never touched." - read -p "Choose 1, 2 or 3 [1]: " src_choice - case "${src_choice}" in - 2) fetch_release_src "${REL}" ;; - 3) fetch_current_src ;; - *) - echo "PFUIDNS: Keeping the existing /usr/src, continuing to the build" - [ "${HAVE_SRC}" = "found" ] \ - || echo "PFUIDNS: WARNING no Makefile.bsd-wrapper under /usr/src; choose 2 or 3 if the build fails" - ;; - esac - elif [[ "$OS" = "FreeBSD" ]]; then # The FreeBSD path never built Unbound (that block is OpenBSD-only) and then # ran the OpenBSD-only tail regardless, so it could only ever half-install. @@ -208,9 +182,55 @@ if [[ "$OS" = "OpenBSD" ]]; then mkdir -p "${TARGET}" fi + # Report what is installed, so the choice below is an informed one. A module + # upgrade is the common case and needs no rebuild. + if [ -x /usr/local/sbin/unbound ]; then + INSTALLED_VER=$(/usr/local/sbin/unbound -V 2>/dev/null | sed -n 's/^Version //p' | head -1) + if /usr/local/sbin/unbound -V 2>/dev/null | grep -q pythonmodule; then + HAVE_UNBOUND="${INSTALLED_VER:-unknown version}, with the Python module" + BUILD_DEFAULT=1 + else + HAVE_UNBOUND="${INSTALLED_VER:-unknown version}, WITHOUT the Python module" + BUILD_DEFAULT=2 + fi + else + HAVE_UNBOUND="none at /usr/local/sbin/unbound" + BUILD_DEFAULT=2 + fi + echo - read -p "Would you like to build Unbound with Python module support (required) y/n: " yn - if [[ "$yn" = "y" ]]; then + echo "Unbound resolver (installed: ${HAVE_UNBOUND})" + echo " 1) keep it, and update only the PFUI module and configuration" + echo " 2) build and install Unbound (${UNBOUND_VERSION}) with the Python module" + read -p "Choose 1 or 2 [${BUILD_DEFAULT}]: " build_choice + [ -n "${build_choice}" ] || build_choice="${BUILD_DEFAULT}" + if [[ "${build_choice}" = "2" ]]; then + # Unbound is built with Makefile.bsd-wrapper taken from the system sources, + # so a usable /usr/src is a prerequisite for the build below + if [ -f /usr/src/usr.sbin/unbound/Makefile.bsd-wrapper ] \ + || [ -f /usr/src/usr.sbin/unbound.base/Makefile.bsd-wrapper ]; then + HAVE_SRC="found" + else + HAVE_SRC="NOT found" + fi + + REL=$(uname -r) + echo "OpenBSD system sources in /usr/src (Unbound's wrapper Makefile comes from there)" + echo " 1) keep the existing tree and carry on to the build [${HAVE_SRC}]" + echo " 2) replace it with the signed ${REL} release sources" + echo " 3) replace it with -current from the git mirror (unsigned)" + echo "Options 2 and 3 delete /usr/src/* first. /usr/ports is never touched." + read -p "Choose 1, 2 or 3 [1]: " src_choice + case "${src_choice}" in + 2) fetch_release_src "${REL}" ;; + 3) fetch_current_src ;; + *) + echo "PFUIDNS: Keeping the existing /usr/src, continuing to the build" + [ "${HAVE_SRC}" = "found" ] \ + || echo "PFUIDNS: WARNING no Makefile.bsd-wrapper under /usr/src; choose 2 or 3 if the build fails" + ;; + esac + RELEASE_HELPER="${DIR}/client-unbound/tools/unbound_release.sh" [ -x "${RELEASE_HELPER}" ] || die "${RELEASE_HELPER} is missing or not executable" echo "PFUIDNS: Resolving which Unbound to build (UNBOUND_VERSION=${UNBOUND_VERSION})" @@ -264,6 +284,16 @@ if [[ "$OS" = "OpenBSD" ]]; then make -f Makefile.bsd-wrapper || die "Unbound ${UNBOUND_REF} failed to build" make install-all || die "Unbound ${UNBOUND_REF} failed to install" make clean + else + echo "PFUIDNS: Keeping the installed Unbound; updating the PFUI module only" + case "${HAVE_UNBOUND}" in + *"WITHOUT the Python module"*) + echo "PFUIDNS: WARNING that Unbound cannot load a Python module, so PFUI" \ + "will not run in it. Re-run and choose 2." ;; + "none at"*) + echo "PFUIDNS: WARNING there is no Unbound to load the module." \ + "Re-run and choose 2." ;; + esac fi echo "PFUIDNS: Installing PFUI_Unbound and Configuration (Python Module for Unbound)"