SIA: reconcile sshd host certificate lines on key type change - #3450
Draft
psasidhar wants to merge 1 commit into
Draft
SIA: reconcile sshd host certificate lines on key type change#3450psasidhar wants to merge 1 commit into
psasidhar wants to merge 1 commit into
Conversation
When the configured ssh host key type changes (e.g. rsa to ecdsa), updateSSH appended the new HostCertificate line and restarted sshd, but left the previous key type's line in place. sshd then kept presenting a certificate that sia was no longer updating, and once that certificate expired sshd could no longer be restarted. updateSSH now reconciles the config in one pass when it adds its HostCertificate line: the lines of the other sia supported host key types (derived from the hostkey package for the same directory) are commented out in the same write. Host certificate lines with custom paths that sia does not manage are never touched. The new line is inserted before the first Match block, if present, since HostCertificate is only valid in the global section. The sshd interaction is also hardened: - the updated config is validated with sshd -t -f before the daemon is asked to re-read it. if validation fails the original config is restored and sshd is left untouched, since reloading into a broken config could lock us out of the instance. sshd is resolved at /usr/sbin/sshd since util.GetUtilPath only searches /usr/bin and /bin - systemctl reload-or-restart replaces restart: sshd re-execs in place without stopping the listener or affecting established sessions, and systemd falls back to a full start if the daemon is not running - the service unit is named sshd on rhel/fedora based systems and ssh on debian/ubuntu (the sshd alias link only exists while the unit is enabled), so both unit names are tried in order - command output is captured into returned errors so failures are actionable from the sia log hostCertificateLinePresent now parses lines by field with a case insensitive keyword match, as sshd does, instead of a prefix match: previously HostCertificate /path/cert.pub.old counted as a match for /path/cert.pub, and lowercase keywords were missed. Steady state behavior is unchanged: when the host certificate line is already present, certificate refreshes do not touch the config file and do not reload sshd. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Sasi Palaka <palakas@yahooinc.com>
psasidhar
marked this pull request as draft
August 13, 2026 17:48
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
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.
Problem
When the configured ssh host key type changes (e.g.
rsa→ecdsa),updateSSHappends the newHostCertificateline to the sshd config and restarts sshd — but the previous key type's line is left in place. sshd keeps presenting a certificate that SIA is no longer refreshing, and once that stale certificate expires, sshd fails to start on the next restart/reboot, locking operators out of the instance.Separately, the restart itself had robustness gaps: the config was never validated before restarting (a bad config bricks sshd),
systemctl restartstops the listener when a reload suffices, and the hardcodedsshdunit name does not exist on debian/ubuntu systems where the unit isssh.Change
Reconcile in one pass. When
updateSSHadds itsHostCertificateline (i.e. the key type changed or this is initial setup), it now also comments out the host certificate lines of the other SIA-supported key types in the same write. The stale path set is derived from thehostkeypackage (hostkey.CertFileforRsa/Ecdsa/Ed25519in the same directory), so:The new line is inserted before the first
Matchblock if one exists, sinceHostCertificateis only valid in the global section (previously a trailingMatchblock would produce an invalid config).Validate, then reload. The updated config is checked with
sshd -t -f <config>before the daemon is asked to re-read it. If validation fails, the original config is restored and the running daemon is left untouched. On success,systemctl reload-or-restartis used — sshd re-execs in place without stopping the listener or affecting established sessions, falling back to a full start if the daemon is not running — trying thesshdunit thenssh(debian/ubuntu; thesshdalias link only exists while the unit is enabled). Command output is captured into returned errors so failures are actionable from the SIA log.Parser fix.
hostCertificateLinePresentpreviously used a prefix match, soHostCertificate /path/cert.pub.oldcounted as a match for/path/cert.pub, and lowercase keywords (valid for sshd) were missed. It now parses by field with a case-insensitive keyword match, as sshd does.Steady state is unchanged: when the certificate line is already present, refreshes do not touch the config file and do not reload sshd.
updateSSH's signature and both call sites are unchanged, and ssh update errors remain non-fatal to the agent.Testing
TestUpdateSSHConfigFile: 7 cases — the 2 original cases (behavior preserved byte-for-byte), rsa→ecdsa, ecdsa→rsa, multiple stale types, custom-path certificate untouched, insertion before aMatchblockTestHostCertificateLinePresent: extended with case-insensitive keyword, tab separator, and partial-path-match cases (the last fails against the previous prefix-match implementation)TestHostCertificateFile,TestSiblingHostCertFiles: new helper coverageTestCheckSshdConfig,TestReloadSshdService: exercised with fake shell-script executables recording their invocations — covers the reload happy path, the debian/ubuntussh-unit fallback, and error aggregation when both units failgo build,go vet, and the fulllibs/go/sia/agentsuite passThe validate-then-reload behavior was also verified end-to-end on a live AWS EC2 instance (AlmaLinux 8.10) via a downstream build: migrating rsa→ecdsa commented out the stale line,
sshd -tgated the reload, and journald showed a true SIGHUP reload with established sessions surviving.Notes
provider/azure/sia-vmhas its own private copy ofrestartSshdServicewith the same unvalidated-restart pattern; updating it the same way is a candidate follow-up, left out to keep this change scoped tolibs/go/sia/agent.🤖 Generated with Claude Code