Skip to content

Fixing RADIUS invalid shell#22933

Merged
yxieca merged 2 commits intosonic-net:masterfrom
dt-nexthop:dt-nexthop/fix-radius-invalid-shell
Jun 12, 2025
Merged

Fixing RADIUS invalid shell#22933
yxieca merged 2 commits intosonic-net:masterfrom
dt-nexthop:dt-nexthop/fix-radius-invalid-shell

Conversation

@dt-nexthop
Copy link
Contributor

Why I did it

This is a follow up to #13141, to include reomving the docker group add for the RO user for RADIUS. The sudoer file already allows docker ps command

Work item tracking
  • Microsoft ADO (number only):

How I did it

How to verify it

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

  • 202205
  • 202211
  • 202305
  • 202311
  • 202405
  • 202411
  • 202505

Tested branch (Please provide the tested image version)

This fix was tested on master branch and 202411 runing the sonic-mgmt RADIUS test suite.

Description for the changelog

Fixed RADIUS user authentication issue due to invalid shell (sonic-launch-shell)
Resolves

This pull request resolves PR#11352 (#11352) under sonic-buildimage repository.

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dt-nexthop
Copy link
Contributor Author

@qiluo-msft @yxieca Here is the follow-up to #13141 and the removal of the docker group for the read-only user. Can you please review it?

@dt-nexthop dt-nexthop changed the title radius invalid shell Fixing RADIUS invalid shell Jun 11, 2025
@qiluo-msft qiluo-msft requested a review from Copilot June 11, 2025 22:00
Copy link
Contributor

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 fixes an issue with the RADIUS user authentication by correcting the shell used for RADIUS users. Key changes include:

  • Removing the docker group assignment for the primary remote user.
  • Changing the shell for both the primary and secondary RADIUS user entries from "/usr/bin/sonic-launch-shell" to "/bin/bash".
Comments suppressed due to low confidence (3)

src/radius/nss/libnss-radius/nss_radius_common.c:160

  • Confirm that removing the docker group from rnm[0] while retaining it in rnm[RADIUS_MAX_MPL-1] is intentional to ensure consistent group permissions across RADIUS user definitions.
rnm[0].groups = "docker"

src/radius/nss/libnss-radius/nss_radius_common.c:162

  • Verify that switching the shell to /bin/bash meets the RADIUS authentication requirements and is available in all intended deployment environments.
rnm[0].shell = "/bin/bash"

src/radius/nss/libnss-radius/nss_radius_common.c:166

  • Ensure that using /bin/bash as the shell for the secondary RADIUS user maintains compatibility with any scripts or commands expecting the previous shell configuration.
rnm[RADIUS_MAX_MPL-1].shell = "/bin/bash"

Copy link

@Yarden-Z Yarden-Z left a comment

Choose a reason for hiding this comment

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

LGTM

@yxieca yxieca merged commit 79e5d74 into sonic-net:master Jun 12, 2025
17 of 19 checks passed
@dt-nexthop dt-nexthop deleted the dt-nexthop/fix-radius-invalid-shell branch July 16, 2025 01:56
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.

7 participants