Skip to content

Fix WireGuard startup error handling - #2377

Merged
openipc-ai merged 8 commits into
OpenIPC:masterfrom
usa-:wireguard-fix
Sep 7, 2026
Merged

Fix WireGuard startup error handling#2377
openipc-ai merged 8 commits into
OpenIPC:masterfrom
usa-:wireguard-fix

Conversation

@usa-

@usa- usa- commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Problem

WireGuard can fail to start after a reboot when the peer endpoint is specified as a hostname and DNS is temporarily unavailable. In that case, the camera can end up without a working WireGuard connection.

Solution

The WireGuard script now waits for DNS to become available when the endpoint is specified as a hostname, while IPv4 and IPv6 literal endpoints skip the DNS wait. An empty wg_endpoint fails immediately with an error.

WireGuard is started asynchronously so that a DNS delay does not block the boot process. rc.local and other boot-time scripts should not assume that wg0 is immediately available.

Hardware tested on

  • gk7205v300, G6S
  • t31x, don't know the board name
  • ssc337de, don't know the board name
  • ssc378de, don't know the board name, multiple cameras

The fix has been tested for more than a week on these platforms.

Evidence

See OpenIPC/firmware#2319 for the problem reproduction, testing results, and discussion.

Scope

  • No kernel patches under general/package/all-patches/linux/ (those go to OpenIPC/linux)
  • No files specific to a single retail camera model (those go to OpenIPC/builder)
  • No probing or bring-up tooling (that goes to OpenIPC/ipctool)
  • Nothing under general/overlay/ or in a shared load_<vendor> script hardcodes a value specific to my board
  • Package sources come from an OpenIPC repository, and any version bump keeps at least the specificity of the pin it replaces (a new package should pin a full 40-character SHA)
  • No LD_PRELOAD, and no binaries that cannot be rebuilt from source
  • New code is selected by a defconfig, so CI actually builds it

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Fix WireGuard startup during temporary DNS outages

🐞 Bug fix 🕐 10-20 Minutes

Grey Divider

AI Description

• Starts WireGuard asynchronously so DNS outages do not block boot initialization.
• Retries hostname resolution every ten seconds before applying the peer configuration.
Diagram

graph TD
  INIT["Init script"] --> KEY{"Private key set?"} -- Yes --> BG["Background startup"] --> TYPE{"Hostname endpoint?"}
  TYPE -- Yes --> DNS["DNS retry loop"] --> CONFIG["Apply WG config"]
  TYPE -- No --> CONFIG
  KEY -- No --> SKIP["Skip startup"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Retry WireGuard configuration directly
  • ➕ Lets WireGuard parse and resolve all supported endpoint formats
  • ➕ Avoids maintaining separate hostname-versus-address detection logic
  • ➖ Repeated configuration attempts may produce noisy failures
  • ➖ Requires careful interface cleanup and retry-state handling
2. Trigger setup from network readiness events
  • ➕ Avoids polling DNS continuously
  • ➕ Aligns tunnel creation with network availability
  • ➖ Requires broader integration with platform-specific network lifecycle scripts
  • ➖ A single readiness event may not cover later DNS recovery

Recommendation: Keep the targeted asynchronous DNS-gating approach because it is small, portable across the tested embedded platforms, and provides eventual recovery without blocking boot. A supervised full-setup retry would be preferable only if other transient startup failures or additional endpoint formats must also be handled.

Files changed (2) +16 / -2

Bug fix (2) +16 / -2
S98wireguardLaunch WireGuard setup asynchronously +1/-1

Launch WireGuard setup asynchronously

• Runs the WireGuard setup command in the background when a private key is configured. This prevents temporary DNS failures from blocking the init sequence.

general/overlay/etc/init.d/S98wireguard

wireguardWait for hostname endpoint resolution +15/-1

Wait for hostname endpoint resolution

• Reads the endpoint once and waits in ten-second intervals for hostname resolution before generating and applying the configuration. Numeric endpoints bypass the DNS wait.

general/overlay/usr/sbin/wireguard

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Sep 6, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Some cameras lose VPN recovery ✓ Resolved 📎 Requirement gap ☼ Reliability
Description
case "$ENDPOINT" classifies endpoints by their first character, mistaking digit-prefixed hostnames
for IPv4 addresses and sending bracketed IPv6 literals—with brackets retained in HOST—to
nslookup. A hostname such as 3cam.example:51820 bypasses DNS retries and can reach a
startup-terminating wg setconf failure during temporary DNS unavailability, while a literal such
as [2001:db8::1]:51820 enters the unterminated resolver retry loop instead of proceeding directly
to configuration.
Code

general/overlay/usr/sbin/wireguard[R10-13]

+case "$ENDPOINT" in
+    [0-9]*)
+        ;;
+    *)
Evidence
Compliance rule 14 requires hostname endpoints to remain recoverable after temporary DNS failure and
literal IP endpoints to start normally. The case expression lets every digit-prefixed endpoint
bypass DNS handling even though the endpoint format does not prohibit digit-leading hostnames, after
which the unchanged configuration reaches wg setconf, whose failure terminates startup.
Conversely, bracketed IPv6 literals begin with [ and therefore enter the wildcard hostname branch;
removing only the final colon-and-port leaves the brackets in HOST, which is passed to nslookup
in a retry loop with no termination condition.

Resilient WireGuard startup during temporary DNS unavailability
general/overlay/usr/sbin/wireguard[10-18]
general/overlay/usr/sbin/wireguard[8-20]
general/overlay/usr/sbin/wireguard[22-35]
general/overlay/etc/wireguard.conf[5-8]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Fix endpoint classification that mistakes digit-prefixed hostnames for IPv4 addresses and bracketed IPv6 literals for hostnames. The current first-character test causes valid digit-leading hostnames to bypass DNS readiness retries, while IPv6 literals are queried with invalid bracketed resolver input and can remain in an unterminated retry loop.
## Issue Context
`wg_endpoint` is emitted directly into the WireGuard configuration, and the repository imposes no restriction against hostnames beginning with a digit. WireGuard endpoint syntax encloses IPv6 literals in brackets when a port is present; hostname endpoints must retry during temporary DNS unavailability, while valid IPv4 and IPv6 literals must proceed without DNS lookup. Use address parsing compatible with the target BusyBox environment, either bypassing readiness checks for literals or parsing and normalizing an address before any lookup.
## Fix Focus Areas
- general/overlay/usr/sbin/wireguard[8-20]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. A resolver relapse leaves tunnels offline 🐞 Bug ☼ Reliability
Description
The loop stops after one successful nslookup, but wg setconf performs a separate hostname
resolution and any failure there exits the script without another outer retry. If DNS briefly
recovers for the probe and becomes unavailable during configuration, the interface remains
unconfigured and no process is left to recover it later.
Code

general/overlay/usr/sbin/wireguard[R16-18]

+        while ! nslookup "$HOST" >/dev/null 2>&1; do
+            sleep 10
+        done
Evidence
The added loop terminates permanently on the first successful lookup. The later wg setconf call
has an exit-on-failure handler, and there is no enclosing retry or cleanup path before the script
terminates.

general/overlay/usr/sbin/wireguard[16-20]
general/overlay/usr/sbin/wireguard[22-35]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A successful readiness probe does not guarantee that the subsequent WireGuard configuration can resolve the endpoint.
## Issue Context
The target failure mode is temporary or unstable DNS during boot. Recovery must retry the operation that actually consumes the hostname, rather than relying on a separate probe followed by a one-shot configuration attempt.
## Fix Focus Areas
- general/overlay/usr/sbin/wireguard[16-35]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Operators cannot cancel pending starts ✓ Resolved 🐞 Bug ☼ Reliability
Description
S98wireguard now backgrounds a potentially unbounded process, but its stop action neither
records nor terminates that process. During a DNS outage, stop-and-start leaves the old waiter alive
while the replacement fails because the old invocation already created wg0, and the old process
can later apply its captured endpoint.
Code

general/overlay/etc/init.d/S98wireguard[7]

+			wireguard &
Evidence
The changed start command backgrounds every invocation, while the service's stop branch is empty.
The worker creates wg0 before entering its unbounded lookup loop, so another start encounters the
explicit interface-creation failure while the original worker continues.

general/overlay/etc/init.d/S98wireguard[3-12]
general/overlay/usr/sbin/wireguard[3-20]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The newly backgrounded startup process has no PID tracking, cancellation, or safe restart behavior.
## Issue Context
The worker creates `wg0` before waiting for DNS and captures configuration before that wait. The service should support stopping a pending worker and retrying without an orphaned process or pre-existing interface conflict.
## Fix Focus Areas
- general/overlay/etc/init.d/S98wireguard[4-12]
- general/overlay/usr/sbin/wireguard[3-20]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. A missing endpoint waits forever ✓ Resolved 🐞 Bug ☼ Reliability
Description
An empty wg_endpoint reaches the wildcard branch, produces an empty HOST, and repeatedly invokes
nslookup without reporting a configuration error. This occurs whenever a private key exists but
the endpoint variable is absent or empty, leaving a background process and an unconfigured wg0
indefinitely.
Code

general/overlay/usr/sbin/wireguard[R14-17]

+        HOST="${ENDPOINT%:*}"
+
+        while ! nslookup "$HOST" >/dev/null 2>&1; do
+            sleep 10
Evidence
The endpoint read has no non-empty validation, and parameter removal leaves an empty value unchanged
when no colon is present. The init script starts the worker based solely on wg_privkey, so this
state is reachable whenever endpoint configuration is missing.

general/overlay/usr/sbin/wireguard[3-18]
general/overlay/etc/init.d/S98wireguard[4-8]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A missing endpoint is treated as a temporarily unresolvable hostname and retried forever.
## Issue Context
The init script only checks for a private key before launching WireGuard. Validate the endpoint separately and fail with a clear error before creating the interface or entering the resolver loop.
## Fix Focus Areas
- general/overlay/usr/sbin/wireguard[3-18]
- general/overlay/etc/init.d/S98wireguard[4-8]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread general/overlay/usr/sbin/wireguard Outdated
Comment thread general/overlay/usr/sbin/wireguard Outdated
Comment thread general/overlay/etc/init.d/S98wireguard
Comment thread general/overlay/usr/sbin/wireguard Outdated

@openipc-ai openipc-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks — the shape is right. A hostname endpoint costs wg setconf about 2.5 minutes of retries at boot and then fails for good, so waiting for the name outside the boot path is the correct fix, and the literal-IPv4 bypass is necessary: busybox nslookup 1.2.3.4 is a reverse lookup and returns 1 even with DNS working. I checked the regex and the nslookup exit codes under the same busybox 1.36.1/musl the image ships. The shell parse and strip tests pass on this head.

One thing to fix before merge (inline): a bracketed IPv6 endpoint never starts. The rest is take-or-leave.

Two points that are not code:

  • Title: the tree uses area: lowercase imperative summary, and maintainers squash with the PR title. wireguard: wait for the endpoint to resolve instead of failing at boot says what changed; "error handling" does not.
  • Evidence: the before-log in #2319 is good. A pasted wg show wg0 from a camera that booted with DNS unreachable and then recovered would close the loop.

On Qodo's review: the digit-prefixed-hostname half of its first finding was fixed by your second commit (the regex sends 3cam.example.com to the resolver correctly). A DNS relapse between the probe and wg setconf is covered by wg's own 15 retries. stop doing nothing, and a second start failing at ip link add, were both true before this change.

For the record, WG_ENDPOINT_RESOLUTION_RETRIES=infinity on wg setconf (the shipped wireguard-tools supports it) was considered: it gives up immediately on NXDOMAIN and REFUSED, which a router's resolver can return while its WAN is down, so your loop is the more robust shape.

CI has not run yet because the fork's workflow runs are waiting for maintainer approval.

Comment thread general/overlay/usr/sbin/wireguard Outdated
Comment thread general/overlay/usr/sbin/wireguard
Comment thread general/overlay/etc/init.d/S98wireguard
Co-authored-by: OpenIPC AI team <ai@openipc.org>
@openipc-ai

Copy link
Copy Markdown
Collaborator

Thanks for applying the fast-fail on an empty wg_endpoint.

Two items from the review are still open on 0622cd3:

  1. IPv6 endpoints (thread) — this is the one blocking approval. A bracketed endpoint such as [2001:db8::1]:51820 fails the IPv4 test, and then nslookup '[2001:db8::1]' returns 1 forever, so a tunnel that comes up today never comes up with this change. Skipping the wait when $HOST starts with [ is enough; the suggestion in that thread does it and can be applied from the web UI like the last one.

  2. PR description (thread) — a sentence noting that wg0 now comes up asynchronously, so rc.local and anything else that runs at boot can no longer assume it exists.

Once the first lands I will re-review.

@openipc-ai openipc-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks, the IPv6 case is fixed. Measured with the new pattern under musl grep -E: [2001:db8::1] and [::1] skip the wait, 1.2.3.4 skips, vpn.example.com and 1abc.example.com wait. Both scripts still parse under busybox sh -n, before and after comment stripping.

The start-stop-daemon change in 0c8e7fe is the new blocker; details and a suggestion to revert it are inline on S98wireguard.

Still open from the first review: a sentence in the description that wg0 now comes up asynchronously, so rc.local and anything else at boot can no longer assume it exists.

CI on this fork is held for maintainer approval; it will be run once the init script is back to its previous form.

Comment thread general/overlay/etc/init.d/S98wireguard Outdated

@openipc-ai openipc-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Everything from both reviews is in: the IPv6 guard, the empty-endpoint fast-fail, S98wireguard back to wireguard & with the empty stop, and the description now says wg0 comes up asynchronously. Both scripts parse under the busybox sh this tree builds, before and after comment stripping.

I brought the branch up to date with master (the protected branch requires it) and released the fork's CI runs. This merges once the board matrix is green.

@openipc-ai
openipc-ai merged commit bf7567a into OpenIPC:master Sep 7, 2026
116 of 117 checks passed
@usa-

usa- commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

I don't think wg setconf should be retried in a loop. It can fail for reasons unrelated to DNS, and retrying it indefinitely would not fix those errors. The DNS wait is only meant to handle the case where the endpoint hostname is not resolvable yet.

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