- Add filterEmptyStrings utility function for consistent string filtering
- Replace inline slices.DeleteFunc calls with filterEmptyStrings
- Apply filtering to osArgs in addition to command args
- Improves code readability and reduces duplication
- Uses slices.DeleteFunc internally for efficient filtering
- Move version checking logic to shouldUpgrade for testability
- Move upgrade command execution to performUpgrade
- selfUpgradeCheck now composes these two for clarity
- Update and expand tests: focus on logic, not side effects
- Improves maintainability, testability, and separation of concerns
Logging there should use Log function to include the request ID if
present. Changes were made unintentionally during the refactoring to
eliminate usage of global logger.
This commits message restores the correct/old behavior.
Split the long proxy method into several smaller methods to improve maintainability
and testability. Each new method has a single responsibility:
- initializeUpstreams: handles upstream configuration setup
- tryCache: manages cache lookup logic
- tryUpstreams: coordinates upstream query attempts
- processUpstream: handles individual upstream query processing
- handleAllUpstreamsFailure: manages failure scenarios
- checkCache: performs cache checks and retrieval
- serveStaleResponse: handles stale cache responses
- shouldContinueWithNextUpstream: determines if failover is needed
- prepareSuccessResponse: formats successful responses
This refactoring:
- Reduces cognitive complexity
- Improves code testability
- Makes the DNS proxy logic flow clearer
- Isolates error handling and edge cases
- Maintains existing functionality
No behavioral changes were made.
This change improves compatibility with newer UniFi OS versions while
maintaining backward compatibility with UniFi OS 4.2 and earlier.
The refactoring also reduces code duplication and improves maintainability
by centralizing dnsmasq configuration path logic.
Break down the large DNS handling function into smaller, focused functions
with clear responsibilities:
- Extract handleDNSQuery from serveDNS handler function
- Create dedicated startListeners function for listener management
- Add standardQueryRequest struct to encapsulate query parameters
- Split special domain handling into separate function
- Add descriptive comments for each new function
- Improve variable names for better clarity (e.g., startTime vs t)
This refactoring improves code maintainability and readability without
changing the core DNS proxy functionality.
Move getDNS type definition from dns.go to os_linux.go where it is used.
Remove the now-empty dns.go file. This change improves code organization
by keeping platform-specific types with their implementations.
Add context parameter to validInterfacesMap for better error handling and
logging. Move Windows-specific network adapter validation logic to the
ctrld package. Key changes include:
- Add context parameter to validInterfacesMap across all platforms
- Move Windows validInterfaces to ctrld.ValidInterfaces
- Improve error handling for virtual interface detection on Linux
- Update all callers to pass appropriate context
This change improves error reporting and makes the interface validation
code more maintainable across different platforms.
Improve documentation for Test_prog_parseResolvConfNameservers to clarify that
the old implementation was removed as part of code deduplication effort. The code
for handling resolv.conf was unified into the resolvconffile package to provide
a consistent interface across the codebase.
This change provides better context for future developers about why the
refactoring was done and what benefits it brings.
Move client information related functions from client_info_*.go to desktop_*.go files
to better organize platform-specific code and separate desktop functionality from
shared code.
No functional changes.
Make nameserver resolution functions more consistent and accessible:
- Rename currentNameserversFromResolvconf to CurrentNameserversFromResolvconf
- Move function to public API for better reusability
- Update all internal references to use the new public API
- Add comprehensive godoc comments for nameserver functions
- Improve code organization by centralizing DNS resolution logic
This change makes the nameserver resolution functionality more maintainable
and easier to use across different parts of the codebase.
By adding a logger field to "prog" struct, and use this field inside its
method instead of always accessing global mainLog variable. This at
least ensure more consistent usage of the logger during ctrld prog
runtime, and also help refactoring the code more easily in the future
(like replacing the logger library).
So setting up logging for ctrld binary and ctrld packages could be done
more easily, decouple the required setup for interactive vs daemon
running.
This is the first step toward replacing rs/zerolog libary with a
different logging library.
This commit reverts changes from v1.4.5 to v1.4.7, to prepare for v2.0.0
branch codes.
Changes includes in these releases have been included in v2.0.0 branch
already.
Details:
Revert "feat: add --rfc1918 flag for explicit LAN client support"
This reverts commit 0e3f764299.
Revert "Upgrade quic-go to v0.54.0"
This reverts commit e52402eb0c.
Revert "docs: add known issues documentation for Darwin 15.5 upgrade issue"
This reverts commit 2133f31854.
Revert "start mobile library with provision id and custom hostname."
This reverts commit a198a5cd65.
Revert "Add OPNsense new lease file"
This reverts commit 7af29cfbc0.
Revert ".github/workflows: bump go version to 1.24.x"
This reverts commit ce1a165348.
Revert "fix: ensure upstream health checks can handle large DNS responses"
This reverts commit fd48e6d795.
Revert "refactor(prog): move network monitoring outside listener loop"
This reverts commit d71d1341b6.
Revert "fix: correct Windows API constants to fix domain join detection"
This reverts commit 21855df4af.
Revert "refactor: move network monitoring to separate goroutine"
This reverts commit 66e2d3a40a.
Revert "refactor: extract empty string filtering to reusable function"
This reverts commit 36a7423634.
Revert "cmd/cli: ignore empty positional argument for start command"
This reverts commit e616091249.
Revert "Avoiding Windows runners file locking issue"
This reverts commit 0948161529.
Revert "refactor: split selfUpgradeCheck into version check and upgrade execution"
This reverts commit ce29b5d217.
Revert "internal/router: support Ubios 4.3+"
This reverts commit de24fa293e.
Revert "internal/router: support Merlin Guest Network Pro VLAN"
This reverts commit 6663925c4d.
Make RFC1918 listener spawning opt-in via --rfc1918 flag instead of automatic behavior.
This allows users to explicitly control when ctrld listens on private network addresses
to receive DNS queries from LAN clients, improving security and configurability.
Refactor network interface detection to better distinguish between physical and virtual
interfaces, ensuring only real hardware interfaces are used for RFC1918 address binding.
- Add UpstreamConfig.VerifyMsg() method with proper EDNS0 support
- Replace hardcoded DNS messages in health checks with standardized verification method
- Set EDNS0 buffer size to 4096 bytes to handle large DNS responses
- Add test case for legacy resolver with extensive extra sections
Move the network monitoring goroutine initialization outside the listener
loop to prevent it from being started multiple times. Previously, the
network monitoring was started once per listener during first run, which
was unnecessary and could lead to multiple monitoring instances.
The change ensures network monitoring is started only once per program
execution cycle, improving efficiency and preventing potential resource
waste from duplicate monitoring goroutines.
- Extract network monitoring goroutine from listener loop
- Start network monitoring once per run cycle instead of per listener
- Maintain same functionality while improving resource usage
The function was incorrectly identifying domain-joined status due to wrong
constant values, potentially causing false negatives for domain-joined machines.
- Move network monitoring initialization out of serveDNS() function
- Start network monitoring in a separate goroutine during program startup
- Remove context parameter from monitorNetworkChanges() as it's not used
- Simplify serveDNS() function signature by removing unused context parameter
- Ensure network monitoring starts only once during initial run, not on reload
This change improves separation of concerns by isolating network monitoring
from DNS serving logic, and prevents potential issues with multiple
monitoring goroutines if starting multiple listeners.
- Add filterEmptyStrings utility function for consistent string filtering
- Replace inline slices.DeleteFunc calls with filterEmptyStrings
- Apply filtering to osArgs in addition to command args
- Improves code readability and reduces duplication
- Uses slices.DeleteFunc internally for efficient filtering
- Move version checking logic to shouldUpgrade for testability
- Move upgrade command execution to performUpgrade
- selfUpgradeCheck now composes these two for clarity
- Update and expand tests: focus on logic, not side effects
- Improves maintainability, testability, and separation of concerns
This change improves compatibility with newer UniFi OS versions while
maintaining backward compatibility with UniFi OS 4.2 and earlier.
The refactoring also reduces code duplication and improves maintainability
by centralizing dnsmasq configuration path logic.
In v1.4.3, ControlD bootstrap DNS is used again for bootstrapping
process. When this happened, the default system nameservers will be
retrieved first, then ControlD DNS will be used if none available.
However, getting default system nameservers process may take longer than
reloading command timeout, causing invalid error message printed.
To fix this, ensuring default system nameservers is retrieved once.
Added more descriptive error messages for TLS certificate verification
failures across DoH, DoT, DoQ, and DoH3 protocols. The error messages
now include:
- Certificate subject information
- Issuer organization details
- Common name of the certificate
This helps users and developers better understand certificate validation
failures by providing specific details about the untrusted certificate,
rather than just a generic "unknown authority" message.
Example error message change:
Before: "certificate signed by unknown authority"
After: "certificate signed by unknown authority: TestCA, TestOrg, TestIssuerOrg"
Fixes search domains not being preserved when the resolv.conf file is
reverted to its previous state. This ensures that important domain
search configuration is maintained during DNS configuration changes.
The search domains handling was missing in setResolvConf function,
which is responsible for restoring DNS settings.
Changed the IPv6 support detection to try multiple common ports (HTTP/HTTPS) instead of
just testing against a DNS port. The function now returns both the IPv6 support status
and the successful port that confirmed the connectivity. This makes the IPv6 detection
more reliable by not depending solely on DNS port availability.
Previously, the function only tested connectivity to a DNS port (53) over IPv6.
Now it tries to connect to commonly available ports like HTTP (80) and HTTPS (443)
until it finds a working one, making the detection more robust in environments where
certain ports might be blocked.
For cached or singleflight messages, the edns0 cookie is currently
shared among all of them, causing mismatch cookie warning from clients.
The ctrld proxy should re-set client cookies for each request
separately, even though they use the same shared answer.
To guard ctrld from possible DoS to remote upstreams, this commit
implements following things:
- Optimizing multiple queries with the same domain and qtype to use
singleflight group, so there's only 1 query to remote upstreams at
any time.
- Adding a hot cache with 1 second TTL, so repeated queries will re-use
the result from cache if existed, preventing unnecessary requests to
remote upstreams.
There's a bug in wmi library which causes race condition when getting
wmi instance manager concurrently. The new tests for setup bootstrap ip
concurrently thus failed unexpectedly.
There's going to be a fix sent to the upstream, in the meantime, disable
the parallel test temporary.
See: https://github.com/microsoft/wmi/issues/165
As part of v1.4.0 release, reading DNS from /etc/resolv.conf file is
only available for Macos. However, there's no reason to prevent this
function from working on other *nix systems.
This commit unify the function to *nix, so it could be added as DNS
source for Linux and Freebsd.
So on system where there's no available DNS, non-ControlD upstreams
could be bootstrapped like before.
While at it, also improving lookupIP to not initializing OS resolver
anymore, removing the un-necessary contention for accquiring/releasing
OS resolver mutex.
If the socket file does not exist, it means that "ctrld start" was never
run. In this case, the warning message should not be printed to avoid
needless confusion.
Generally, using /jffs/scripts/dnsmasq.postconf is the right way to add
custom configuration to dnsmasq on Merlin. However, we have seen many
reports that the postconf does not work on their devices.
This commit changes how dnsmasq config manipulation is done on Merlin,
so it's expected to work on all Merlin devices:
- Writing /jffs/scripts/dnsmasq.postconf script
- Copy current dnsmasq.conf to /jffs/configs/dnsmasq.conf
- Run postconf script directly on /jffs/configs/dnsmasq.conf
- Restart dnsmasq
This way, the /jffs/configs/dnsmasq.conf will contain both current
dnsmasq config, and also custom config added by ctrld, without worrying
about conflicting, because configuration was added by postconf.
See (1) for more details about custom config files on Merlin.
(1) https://github.com/RMerl/asuswrt-merlin.ng/wiki/Custom-config-files
So using "ctrld stop" or service manager to stop ctrld will end up with
the same result, stopped ctrld with a working DNS, and deactivation pin
code will always have effects if set.
Since requests are mostly originated from the machine itself, so all
necessary metadata is local to it.
Currently, the desktop platforms are Windows desktop and darwin.
netmon provides ipv6 availability during network event changes, so use
this metadata instead of wasting on polling check.
Further, repeated network errors will force marking ipv6 as disable if
were being enabled, catching a rare case when ipv6 were disabled from
cli or system settings.
Currently, custom config is only validated against invalid syntax, not
the validating rules for each configuration value. It causes ctrld
process fatal instead of disregarding as expected.
To fix this, force the validating rule after fetching remote config.
While at it, also add the default network value if non-existed.
When ctrld performs upgrading tasks, the current binary would be moved
to different file, thus the executable will return this new file name,
instead of the old "/path/to/ctrld".
The config path on FreshTomato is located in the same directory with
ctrld binary, with ".startup" suffix. So when the binary was moved
during upgrading, the config path is located wrongly.
To fix it, read the binary path from service config first, then only
fallback to the current executable if the path is empty (this is the
same way ctrld is doing for other router platforms).
So the "ctrld start" should know earlier that "ctrld run" failed to
listen on certain port, and terminate earlier instead of waiting for
timeout happened.
Since application may need SRV record for public domains, which could be
blocked by OS resolver, but not with remote upstreams.
This was reported by a Minecraft user, who seeing thing is broken after
upgrading to v1.4.0 release.
The log ip connection may be nil, since it was not created if blocked by
firewall/VPN apps.
While at it, also add warning when the ipc connection could not be created.
Currently, ctrld requires the default route interface existed to be
functional correctly.
However, on systems where default route is non existed, or point to a
virtual interface (like ipsec based VPN), the fact that the OS is using
this interface as default gateway and doesn't actually send things to
127.0.0.1 is not ctrld's problem.
In this case, ctrld should just start normally, without worrying about
the no default route interface problem.
The runtime internal log should be initialized right after normal log
from configuration, prevent missing log from any actions that could be
happened between two initializations.
For partial init log data (does not end with a newline), the log writer
discard data after the last newline to make the log prettier, then write
the init end marker. This causes the marker could be written more than
once, since the second overflows will preserve the data which does
include the marker from the first write.
To fix this, ensure that the init end marker is only written once, and
the second overflows will preserve data until the marker instead of the
fixed initial size like the first one.
linux/amd64 have the nocgo binary to support system where standard libc
missing.
If the current binary is a nocgo version, "ctrld upgrade" command must
honor the nocgo setting and download the right binary.
openwrt 24.10 changes the dnsmasq default config path, causing breaking
changes to softwares which depends on old behavior.
This commit adds a workaround for the issue, by querying the actual
config directory from ubus service list, instead of relying on the
default hardcode one.
Currently, the router discovers initialization are done during startup.
If it were failed, the discovers are skipped. This is too strict, since
the initialization could be failed due to some requires services are not
ready when ctrld started, or router specific requirements for services
management during startup (like UnifiOS v4.0.20).
To fix this, ctrld should relax the initialization checking, allow it to
be failed, and still use the discovers later.
Because ctrld needs to query custom client mapping from it.
While at it, also make the error message clearer when initializing ubios
discover failed, by attaching the command output to returned error.
In case of remote config validation error during start, it's likely that
there's problem with connecting to ControlD API. The ctrld daemon was
restarted in this case, but may not ready to receive requests yet.
This commit changes the error message to explicitly state that instead
of a mis-leading "could not complete service restart".
Previously, a valid interfaces map is only meaningful on Windows and
Darwin, where ctrld needs to set DNS for all physical interfaces.
With new network monitor, the valid interfaces is used for checking new
changes, thus we have to implement the valid interfaces map for all
systems.
- On Linux, just retrieving all non-virtual interfaces.
- On others, fallback to use default route interface only.
The ProxyLogger must only be set after mainLog is fully initialized.
However, it's being set before the final initialization of mainlog,
causing it still refers to stale old pointer.
To fix this, introduce a new function to discard ProxyLogger explicitly,
and use this function to init logging for all interactive commands.
By using existed package instead of hand written one.
While at it, also simplifying the logger getter, since the ProxyLogger
is guaranted to be non-nil.
Cuong Manh Le