Fix silently lost configuration updates and ordered-by user lists - #630
Merged
Conversation
nc_server_config_setup_data() rebuilds the configuration from scratch, so Call Home client thread data is NULL even for running clients. Reconciling then dispatched a second thread and leaked the first one's data. Carry running client thread data over to the new configuration by name before deciding whether to dispatch. Also clear the Call Home dispatch data in nc_server_destroy(), its callback data does not have to be valid once the server is destroyed.
The config lock is held in read mode during the whole transport handshake (nc_accept(), Call Home thread). SSH/TLS key exchange and authentication have their own timeouts (10s, 30s, or even unlimited), so a reader often holds the lock longer than NC_CONFIG_LOCK_TIMEOUT. Applying a config then timed out on the write lock and failed silently. Wait longer than any handshake when applying a new config and report every timeout.
"endpoints/endpoint" and "host-key" are ordered-by user and their order matters - CH endpoints are tried in sequence and the hostkey order decides the advertised algorithm. The order was ignored so far: entries were appended and a delete swapped the last entry into the freed slot. On top of that, moving an entry is reported as a replace in the diff, which no handler covered and left the entry NULL, crashing on its name. Find the existing entry on a replace and place a created or moved entry at the position from the diff "key" metadata, shifting the rest on delete. Also reject unexpected operations in the other find/create handlers.
michalvasko
requested changes
Aug 27, 2026
michalvasko
approved these changes
Aug 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #627.
A configuration apply needs the config write lock, but that lock is held in read mode across entire transport handshakes, which can legitimately take longer than the 10s apply timeout (
auth-timeoutdefaults to 30s and may be 0 = unbounded). The apply then timed out and failed without logging anything, so a newly configured Call Home client was never dispatched.endpointandhost-keyordered-by user lists, which also fixes a crash when an entry is moved (reported as a replace)Instead of discarding the change when a handshake is already in progress, this update forces the apply action to wait for the handshake to complete. Removing that wait entirely is the follow-up PR.