Skip to content

Conversation

@bhouse-nexthop
Copy link
Contributor

@bhouse-nexthop bhouse-nexthop commented Apr 11, 2025

Why I did it

The SSH configuration does not contain many of the hardening requirements by the various standards bodies.

This PR depends on sonic-net/sonic-host-services#238
Fixes #22309

How I did it

This adds support for:

  • password_authentication - ability to disable password auth
  • permit_root_login - ability to prevent root logins
  • ciphers - ability to specify available ciphers
  • kex_algorithms - ability to specify key exchange algorithms
  • macs - ability to specify macs

How to verify it

sonic-yang-models runs tests during build, as does sonic-host-services which validates the behavior.

Which release branch to backport (provide reason below if selected)

  • 202411

Tested branch (Please provide the tested image version)

master as of 20250410

Description for the changelog

SSH hardening configuration options

Link to config_db schema for YANG module changes

https://github.com/bhouse-nexthop/sonic-buildimage/blob/bhouse-nexthop/ssh-config/src/sonic-yang-models/doc/Configuration.md#ssh_server

A picture of a cute animal (not mandatory but encouraged)

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jeff-yin
Copy link
Collaborator

jeff-yin commented May 15, 2025

The original issue #22309 states:

The current SONiC SSH default configuration will not pass most security scanners

Are we updating the SSH default config? I don't see code in this PR or sonic-net/sonic-host-services#238 that indicates changes to the default SSH configs.

If we need to update the default cipher/kex/mac algorithms, it may be helpful to highlight the exact "hardening requirements by the various standards bodies" we're trying to satisfy.

Or are we just going with the existing OpenSSH defaults (i.e., whatever settings are done when no value is specified for the given setting)?

@bhouse-nexthop
Copy link
Contributor Author

No defaults are updated at all. Just enable the configuration knobs so that a user can comply with whatever security standard they're using. I took the approach of not potentially breaking users.

@bhouse-nexthop bhouse-nexthop force-pushed the bhouse-nexthop/ssh-config branch from 256c269 to 2a6b798 Compare May 20, 2025 12:34
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bhouse-nexthop bhouse-nexthop force-pushed the bhouse-nexthop/ssh-config branch from 2a6b798 to 5cd4ba0 Compare May 20, 2025 14:43
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bhouse-nexthop bhouse-nexthop force-pushed the bhouse-nexthop/ssh-config branch from 5cd4ba0 to 4165c41 Compare June 17, 2025 14:45
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@bhouse-nexthop
Copy link
Contributor Author

I just rebased against latest master to force a rebuild since there were spurious failures.

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bhouse-nexthop bhouse-nexthop force-pushed the bhouse-nexthop/ssh-config branch from 4165c41 to d1c930e Compare June 18, 2025 22:50
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bradh352
Copy link
Contributor

I rebased again to force tests to run, they are all passing now.

@qiluo-msft qiluo-msft requested review from Copilot and liuh-80 June 19, 2025 18:17
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds SSH hardening configuration options to the sonic-ssh-server YANG model, updates tests and sample configs to cover them, and refreshes documentation accordingly.

  • Extended YANG model with new leaves and leaf-lists for root login policy, password authentication toggle, and allowed ciphers/kex/mac algorithms
  • Updated test configurations and metadata to validate valid and invalid values for the new options
  • Refreshed sample_config_db.json and Configuration.md to include the new SSH settings

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/sonic-yang-models/yang-models/sonic-ssh-server.yang Added permit_root_login, password_authentication, ciphers, kex_algorithms, and macs
src/sonic-yang-models/tests/yang_model_tests/tests_config/ssh-server.json Added new test cases and invalid-value entries for the SSH hardening options
src/sonic-yang-models/tests/yang_model_tests/tests/ssh-server.json Added metadata entries for the new permit_root_login scenarios
src/sonic-yang-models/tests/files/sample_config_db.json Updated example config to include the new SSH policies
src/sonic-yang-models/doc/Configuration.md Documented the new options and updated the sample snippet
Comments suppressed due to low confidence (3)

src/sonic-yang-models/yang-models/sonic-ssh-server.yang:62

  • Consider specifying a default value (e.g., default "prohibit-password") for permit_root_login to ensure deterministic behavior and align with the documentation.
                leaf permit_root_login {

src/sonic-yang-models/doc/Configuration.md:2952

  • Fix the spelling of 'seperated' to 'separated'.
-   ports - Ssh port numbers - string of port numbers seperated by ','

src/sonic-yang-models/doc/Configuration.md:2973

  • The example uses "false" for permit_root_login, which isn't one of the defined enum values. Use a valid option like "no" or the documented default "prohibit-password".
            "permit_root_login": "false",

qiluo-msft
qiluo-msft previously approved these changes Jun 19, 2025
@prabhataravind
Copy link
Contributor

@bhouse-nexthop could you fix conflicts on this so @qiluo-msft can merge?

@qiluo-msft
Copy link
Collaborator

@bhouse-nexthop This branch has conflicts that must be resolved

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bhouse-nexthop
Copy link
Contributor Author

@qiluo-msft merge conflicts have been resolved

The SSH configuration does not contain many of the hardening
requirements by the various standards bodies.  This adds
support for:
 * password_authentication - ability to disable password auth
 * permit_root_login - ability to prevent root logins
 * ciphers - ability to specify available ciphers
 * kex_algorithms - ability to specify key exchange algorithms
 * macs - ability to specify macs

Signed-off-by: Brad House <[email protected]>
@bhouse-nexthop bhouse-nexthop force-pushed the bhouse-nexthop/ssh-config branch from a13d619 to 835f191 Compare October 24, 2025 14:39
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bradh352
Copy link
Contributor

another spurious test failure unrelated to the changes. Rebased to force a rebuild.

@bhouse-nexthop
Copy link
Contributor Author

@lguohan this is the yang model update for the ticket you just merged :)

@bhouse-nexthop
Copy link
Contributor Author

@lguohan can this be merged?

@lguohan lguohan merged commit d628a99 into sonic-net:master Oct 25, 2025
21 checks passed
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.

Enhancement: SSH configuration hardening

7 participants