hostcfgd: support configurable SSH listen_addresses by pkawatkar14 · Pull Request #431 · sonic-net/sonic-host-services

pkawatkar14 · 2026-09-02T18:40:55Z

Why I did it

Implements Feature 29390131: allow operators to explicitly restrict the SSH daemon to a specific set of assigned addresses via SSH_SERVER|POLICIES > listen_addresses, instead of always listening on all interfaces (wildcard).

How I did it

get_dut_ip_addresses(): enumerate all IPv4/IPv6 addresses currently assigned to any interface (management, loopback, VLAN, etc.).
handle_listen_addresses_set(): dedicated multi-value handler (mirrors handle_ports_set()). Validates syntax, rejects duplicates, and rejects the entire update with an actionable syslog error if any configured address is not currently assigned to the DUT — before touching the temporary sshd config.
restore_default_listen_addresses(): restores ListenAddress 0.0.0.0 / ListenAddress :: wildcards when listen_addresses is removed (field deleted alone, or the whole POLICIES row deleted).
All active/commented ListenAddress directives are removed and replaced atomically in the global config section (before any Match block); validated via sshd -T before the config swap, so a rejected update never touches the working sshd_config or restarts SSH.
Management interfaces are not required — devices without a mgmt address (e.g. M0/MX) are supported since only assigned loopback/VLAN addresses can be configured.

Review feedback addressed

listen_addresses_configured is now initialized from the on-disk sshd_config (not always False), so a hostcfgd restart with a stale restricted ListenAddress and no listen_addresses in CONFIG_DB still restores the wildcard listeners instead of leaving the stale subset in place.
The ListenAddress insertion anchor is now computed from the existing ListenAddress/#ListenAddress line (falling back to the first Match block, or EOF) before deleting those lines, instead of relying on a fallback that produced a meaningless line number when no ListenAddress line existed.
The post-validation sshd restart failure path now logs an explicit, actionable message when listen_addresses was part of the update, since sshd -T only validates syntax and can't catch an address that became unassigned between validation and reload.

How to verify it

Unit tests added in tests/hostcfgd/hostcfgd_ssh_server_test.py / test_ssh_server_vectors.py covering: default values, IPv4-only, IPv6-only, combined IPv4+IPv6, multiple addresses, replacing existing explicit addresses, removal/row-deletion restoring wildcards, invalid/unassigned/duplicate address rejection, and one-unassigned-address rejecting the whole update.

Also validated end-to-end on a live KVM vms-kvm-t0 testbed: hostcfgd correctly binds sshd to exactly the configured addresses, SSH succeeds via configured addresses, and removing listen_addresses restores wildcard listeners.

Related PRs:

sonic-net/sonic-buildimage (YANG model changes)
sonic-net/sonic-mgmt (integration test)

Implement runtime handling for the new SSH_SERVER|POLICIES listen_addresses field: - get_dut_ip_addresses(): enumerate all IPv4/IPv6 addresses (management, loopback, VLAN, etc.) currently assigned to any interface on the DUT. - handle_listen_addresses_set(): dedicated multi-value handler (mirrors handle_ports_set()) that validates syntax, rejects duplicates, and rejects the entire update with an actionable syslog error if any configured address is not currently assigned to the DUT - before modifying the temporary sshd config. - restore_default_listen_addresses(): restores 'ListenAddress 0.0.0.0' and 'ListenAddress ::' wildcards when listen_addresses is removed, whether by deleting just the field or the entire POLICIES row. - All active/commented ListenAddress directives are removed and replaced atomically in the global section (before any Match block); validated via 'sshd -T' before the config swap, so a rejected update never touches the working sshd_config or restarts SSH. Add unit tests covering default values, IPv4-only, IPv6-only, combined, multiple addresses, replacing existing explicit addresses, removal/row deletion restoring wildcards, invalid/unassigned/duplicate address rejection, and one-unassigned-address rejecting the whole update. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Pooja Kawatkar <pkawatkar@microsoft.com>

linux-foundation-easycla · 2026-09-02T18:41:02Z

The committers listed above are authorized under a signed CLA.

✅ login: Copilot / name: Copilot (5009f71)
✅ login: pkawatkar14 / name: Pooja Kawatkar (5009f71)

azure-pipelines · 2026-09-02T18:41:04Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

mssonicbld · 2026-09-02T18:41:14Z

/azp run

azure-pipelines · 2026-09-02T18:41:24Z

Azure Pipelines: Successfully started running 1 pipeline(s).

mssonicbld · 2026-09-03T01:52:31Z

Hi, there are workflow run(s) waiting for approval, you may be first-time contributor. I will notify maintainers to help approve once PR is approved. Thanks!

_{---Powered by SONiC BuildBot}

pkawatkar14 · 2026-09-03T19:32:32Z

/azpw run Azure.sonic-buildimage

mssonicbld · 2026-09-03T19:32:34Z

⚠️ Notice: /azpw run only runs failed jobs now. If you want to trigger a whole pipline run, please rebase your branch or close and reopen the PR.
💡 Tip: You can also use /azpw retry to retry failed jobs directly.

Retrying failed(or canceled) jobs...

mssonicbld · 2026-09-03T19:32:37Z

Retrying failed(or canceled) stages in build 1209961:

✅Stage Build:

Job Build job: retried.

qiluo-msft · 2026-09-03T21:18:42Z

+        assigned_addresses = get_dut_ip_addresses()
+        for addr in canonical_addresses:
+            if addr in SSH_DEFAULT_LISTEN_ADDRESSES:
+                continue


nit: if an operator explicitly sets listen_addresses: ["0.0.0.0", "::"] the wildcards pass through silently — the behavior is identical to not setting listen_addresses at all, so the field has no visible effect. Worth a LOG_WARNING syslog line here so operators who think they have restricted something get feedback:

if addr in SSH_DEFAULT_LISTEN_ADDRESSES: syslog.syslog(syslog.LOG_WARNING, "Ssh listen_addresses includes wildcard {} — equivalent to default (all interfaces)".format(addr)) continue

Feel free to ignore if the design intent is to silently allow wildcards.

I added commit ad8b667 which now logs a LOG_WARNING ("Ssh listen_addresses includes wildcard {} - equivalent to default (all interfaces)") when a configured address is a wildcard, so operators get explicit feedback rather than a silent no-op.

Address PR review feedback: explicitly configuring listen_addresses with only 0.0.0.0/:: is functionally identical to not setting the field at all. Log a LOG_WARNING syslog line so operators get feedback that no addresses were actually restricted, instead of silently behaving like the default. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Pooja Kawatkar <pkawatkar@microsoft.com>

mssonicbld · 2026-09-11T11:48:38Z

/azp run

azure-pipelines · 2026-09-11T11:48:47Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Follow-up to the SSH listen_addresses min-elements PR discussion: since YANG intentionally allows an empty leaf-list, log an explicit LOG_WARNING syslog line when hostcfgd rejects it as unusable, rather than silently falling through to the generic error path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Pooja Kawatkar <pkawatkar@microsoft.com>

mssonicbld · 2026-09-11T11:52:05Z

/azp run

azure-pipelines · 2026-09-11T11:52:14Z

Azure Pipelines: Successfully started running 1 pipeline(s).

pkawatkar14 · 2026-09-11T12:05:27Z

/azpw run Azure.sonic-buildimage

mssonicbld · 2026-09-11T12:05:30Z

⚠️ Notice: /azpw run only runs failed jobs now. If you want to trigger a whole pipline run, please rebase your branch or close and reopen the PR.
💡 Tip: You can also use /azpw retry to retry failed jobs directly.

Retrying failed(or canceled) jobs...

mssonicbld · 2026-09-11T12:05:32Z

Retrying failed(or canceled) stages in build 1217823:

✅Stage Build:

Job Build job: retried.

test_create_checkpoint's last sub-case replaced os.remove directly (os.remove = mock_remove) instead of using a scoped mock.patch like every other mock in this file. Since it was never restored, the real os.remove stayed broken for the remainder of the pytest session, silently poisoning any later test that relied on a real filesystem os.remove() call - including the new hostcfgd SSH listen_addresses tests added for Feature 29390131, causing 5 unrelated CI failures on PR sonic-net#431: tests/hostcfgd/hostcfgd_ssh_server_test.py::TestHostcfgdSSHServerListenAddresses::* Replace the unscoped assignment with mock.patch("ssh_mgmt.os.remove", side_effect=mock_remove), consistent with the rest of the file, so the patch is properly torn down at the end of the with-block. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Pooja Kawatkar <pkawatkar@microsoft.com>

mssonicbld · 2026-09-11T13:55:50Z

/azp run

azure-pipelines · 2026-09-11T13:56:01Z

Azure Pipelines: Successfully started running 1 pipeline(s).

…sses and listen_addresses edge cases Why I did it PR sonic-net#431's coverage check was failing: scripts/hostcfgd diff coverage was 76% (63 lines, 15 missing), below the required 80% threshold. Missing lines were 286-290, 292-297 (get_dut_ip_addresses, which was entirely mocked out in existing tests) and 1143-1144, 1169-1170 (empty-list rejection and wildcard-exemption branches in handle_listen_addresses_set). How I did it Added tests/hostcfgd/hostcfgd_ssh_server_test.py: - test_listen_addresses_empty_list_rejected: verifies handle_listen_addresses_set([]) returns False, leaves config unchanged, and logs a warning. - test_listen_addresses_wildcard_mixed_with_assigned_address: verifies a wildcard address (0.0.0.0) mixed with a real assigned address is accepted and logs the 'equivalent to default' warning. - New TestGetDutIpAddresses class with 4 tests exercising the real get_dut_ip_addresses() function directly (mocking only hostcfgd.psutil.net_if_addrs): IPv4/IPv6 addresses returned, non-IP families ignored, IPv6 zone-id suffix stripped, and unparsable addresses skipped. How to verify it Ran the full sonic-host-services test suite (pytest tests/) inside a sonic-slave-trixie container with python3-swsscommon installed: 774 passed, scripts/hostcfgd overall coverage 84%. Confirmed the previously missing diff lines (286-297, 1143-1144, 1169-1170) are now exercised via targeted --cov-report=term-missing runs. Signed-off-by: Pooja Kawatkar <pkawatkar@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Pooja Kawatkar <pkawatkar@microsoft.com>

mssonicbld · 2026-09-11T17:52:19Z

/azp run

azure-pipelines · 2026-09-11T17:52:29Z

Azure Pipelines: Successfully started running 1 pipeline(s).

mssonicbld · 2026-09-15T12:54:15Z

/azp run

azure-pipelines · 2026-09-15T12:54:25Z

Azure Pipelines: Successfully started running 1 pipeline(s).

- Init listen_addresses_configured from the on-disk sshd_config instead of always False, so a hostcfgd restart with a stale restricted ListenAddress and no listen_addresses in CONFIG_DB still restores the wildcard listeners instead of leaving the stale subset in place. - Compute the ListenAddress insertion anchor from the existing ListenAddress/#ListenAddress line (or the first Match block, or EOF) before deleting those lines, instead of relying on get_line_num_of_pattern's fallback, which produced a meaningless line number when no ListenAddress line existed. - Log an explicit, actionable message when the post-validation sshd restart fails while listen_addresses was part of the update, since sshd -T only validates syntax and won't catch an address that became unassigned between validation and reload. Signed-off-by: Pooja Kawatkar <pkawatkar@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

mssonicbld · 2026-09-15T14:10:55Z

/azp run

azure-pipelines · 2026-09-15T14:11:05Z

Azure Pipelines: Successfully started running 1 pipeline(s).

…ace logic An empty listen_addresses list is syntactically valid per YANG (no min-elements constraint), but previously any single invalid listen_addresses value - including an empty list - aborted the whole SSH policy update, dropping unrelated bundled changes (ports, ciphers, timeout, etc.) too. - set_policies now treats an empty listen_addresses list as a no-op for that field only: logs a warning and keeps the previously applied listen_addresses state untouched, while still applying the rest of the batch. Other validation failures (invalid IP, duplicate, unassigned address) are unchanged and still abort the whole update. - Extracted the shared "drop existing ListenAddress lines, insert new ones" logic from handle_listen_addresses_set and restore_default_listen_addresses into a single _replace_listen_addresses helper. - Added a regression test verifying an empty listen_addresses bundled with another policy change no longer blocks that other change. Signed-off-by: Pooja Kawatkar <pkawatkar@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

mssonicbld · 2026-09-18T15:42:41Z

/azp run

azure-pipelines · 2026-09-18T15:42:51Z

Azure Pipelines: Successfully started running 1 pipeline(s).

mssonicbld · 2026-09-18T18:23:18Z

Hi @sonic-net/sonic-host-services-maintainer, this approved PR has workflow run(s) waiting for approval. Please help review. Thanks!

_{---Powered by SONiC BuildBot}

mssonicbld · 2026-09-19T00:22:30Z

Hi @sonic-net/sonic-host-services-maintainer, this approved PR has workflow run(s) waiting for approval. Please help review. Thanks!

_{---Powered by SONiC BuildBot}

mssonicbld · 2026-09-19T06:22:18Z

Hi @sonic-net/sonic-host-services-maintainer, this approved PR has workflow run(s) waiting for approval. Please help review. Thanks!

_{---Powered by SONiC BuildBot}

mssonicbld · 2026-09-19T12:22:02Z

Hi @sonic-net/sonic-host-services-maintainer, this approved PR has workflow run(s) waiting for approval. Please help review. Thanks!

_{---Powered by SONiC BuildBot}

pkawatkar14 mentioned this pull request Sep 2, 2026

ssh: add integration test for configurable listen_addresses sonic-net/sonic-mgmt#27679

Open

pkawatkar14 mentioned this pull request Sep 2, 2026

sonic-yang-models: add optional listen_addresses to SSH_SERVER|POLICIES sonic-net/sonic-buildimage#29311

Open

qiluo-msft reviewed Sep 3, 2026

View reviewed changes

qiluo-msft reviewed Sep 11, 2026

View reviewed changes

Comment thread scripts/hostcfgd

qiluo-msft reviewed Sep 11, 2026

View reviewed changes

Comment thread scripts/hostcfgd

qiluo-msft reviewed Sep 11, 2026

View reviewed changes

Comment thread scripts/hostcfgd Outdated

pkawatkar14 force-pushed the dev_ado29390131_pk branch from b4bdafe to 2b1fc2a Compare September 15, 2026 14:10

qiluo-msft reviewed Sep 15, 2026

View reviewed changes

Comment thread scripts/hostcfgd

qiluo-msft approved these changes Sep 15, 2026

View reviewed changes

Conversation

pkawatkar14 commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why I did it

How I did it

Review feedback addressed

How to verify it

Uh oh!

linux-foundation-easycla Bot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azure-pipelines Bot commented Sep 2, 2026

Uh oh!

mssonicbld commented Sep 2, 2026

Uh oh!

azure-pipelines Bot commented Sep 2, 2026

Uh oh!

mssonicbld commented Sep 3, 2026

Uh oh!

pkawatkar14 commented Sep 3, 2026

Uh oh!

mssonicbld commented Sep 3, 2026

Uh oh!

mssonicbld commented Sep 3, 2026

Uh oh!

qiluo-msft Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

pkawatkar14 Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Sep 11, 2026

Uh oh!

azure-pipelines Bot commented Sep 11, 2026

Uh oh!

mssonicbld commented Sep 11, 2026

Uh oh!

azure-pipelines Bot commented Sep 11, 2026

Uh oh!

pkawatkar14 commented Sep 11, 2026

Uh oh!

mssonicbld commented Sep 11, 2026

Uh oh!

mssonicbld commented Sep 11, 2026

Uh oh!

mssonicbld commented Sep 11, 2026

Uh oh!

azure-pipelines Bot commented Sep 11, 2026

Uh oh!

mssonicbld commented Sep 11, 2026

Uh oh!

azure-pipelines Bot commented Sep 11, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mssonicbld commented Sep 15, 2026

Uh oh!

azure-pipelines Bot commented Sep 15, 2026

Uh oh!

mssonicbld commented Sep 15, 2026

Uh oh!

azure-pipelines Bot commented Sep 15, 2026

Uh oh!

Uh oh!

mssonicbld commented Sep 18, 2026

Uh oh!

azure-pipelines Bot commented Sep 18, 2026

Uh oh!

mssonicbld commented Sep 18, 2026

Uh oh!

mssonicbld commented Sep 19, 2026

Uh oh!

mssonicbld commented Sep 19, 2026

Uh oh!

mssonicbld commented Sep 19, 2026

Uh oh!

Reviewers

Assignees

pkawatkar14 commented Sep 2, 2026 •

edited

Loading

linux-foundation-easycla Bot commented Sep 2, 2026 •

edited

Loading

pkawatkar14 Sep 11, 2026 •

edited

Loading