[libnss-radius] Handle empty secondary groups for useradd#23571
[libnss-radius] Handle empty secondary groups for useradd#23571manoharan-nexthop wants to merge 1 commit intosonic-net:masterfrom
Conversation
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@qiluo-msft, would you mind reviewing this when you get a chance? |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug in the libnss-radius NSS module where user creation fails when the secondary groups list is empty. The issue occurs during user authentication when useradd is called with an invalid -G argument containing an empty string, causing the user creation to fail and subsequent authentication to use incorrect information.
- Adds validation to check if secondary groups string is non-empty before passing it to
useradd - Provides separate
execlcalls for cases with and without secondary groups - Fixes the authentication flow by ensuring user creation succeeds before authentication attempts
|
Could you add sonic-mgmt testcase? |
|
Yes, there are in this PR - sonic-net/sonic-mgmt#16475 - but since the current master build in sonic has this bug, all the tests will fail on the sonic-mgmt PR. We have run them locally, though, with this patch, and they do pass. @manoharan-nexthop, can you paste in the local results to this PR? |
With this change, all the tests passes.. 1 test_radius_rw_user PASS |
|
@qiluo-msft please give another review to this PR when you get a chance |
Why I did it User authentication through radius fails with invalid password information sent to radius server. How I did it When a user login is attempted, the user is created locally and then the user is processed for authentication. With the issue, the user creation fails and the successive authentication happens with wrong information. When the user was created locally and then the authentication was attempted, we dont hit the issue. the user addition fails as the secondary group list is empty and when it is, useradd fails with invalid -G argument. Check for secondary group string validity before using the same. How to verify it Verified the user authentication is successful.
manoharan-nexthop
force-pushed
the
nss-radius
branch
from
3b6447e to
1154aa8
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Yarden-Z
left a comment
Yarden-Z
left a comment
There was a problem hiding this comment.
Looks good - do we have a fix for tacacs as well?
Since I think the same item applies there as well
|
@Yarden-Z, not sure about TACACS, can we take that up in a different PR? |
I dont see similar issue in TACACS.. The issue is happening only with RADIUS. |
|
@ Yarden-Z I wanted to see if there is anything left we need to do merge this one in. I would like to get this merged in so the RADIUS test in sonic-mgmt will also start passing. |
Yarden-Z
left a comment
Yarden-Z
left a comment
There was a problem hiding this comment.
If nothing is relevant for Tacacs - this looks ok.
Fixes #23570
How I did it
When a user login is attempted, the user is created locally and then the user is processed for authentication. With the issue, the user creation fails and the successive authentication happens with wrong information. When the user was created locally and then the authentication was attempted, we dont hit the issue.
the user addition fails as the secondary group list is empty and when it is, useradd fails with invalid -G argument. Check for secondary group string validity before using the same.
How to verify it
Verified the user authentication is successful.
Which release branch to backport (provide reason below if selected)
The issue exists only in
master.Description for the changelog
[libnss-radius] Handle empty secondary groups for useradd