-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RADIUS Management User Authentication Feature #7284
Conversation
HLD: https://github.com/Azure/SONiC/blob/master/doc/aaa/radius_authentication.md UT: src/sonic-host-services/tests/hostcfgd/hostcfgd_radius_test.py CLI: In a separate PR.
This pull request introduces 3 alerts when merging 3f26e3e into e30a7eb - view on LGTM.com new alerts:
|
This pull request introduces 3 alerts when merging d1a7a26 into 75c29cb - view on LGTM.com new alerts:
|
LGTM Warnings UT harness fixes
/azp run |
Commenter does not have sufficient privileges for PR 7284 in repo Azure/sonic-buildimage |
src/sonic-host-services-data/debian/sonic-host-services-data.aaastatsd.service
Show resolved
Hide resolved
- Address review comments - Renamed tacacs_get_source_intf_ip() to be more generic - Removed redundant run_cmd() definition. Added return in exception hdlr. - Removed redundant parameter in handle_radius_source_intf_ip_chg() - src_ip is deprecated. Honor an attempt to configure src_intf, even if it does not yield an IP address. - Remove the modification of sudo PAM file - Adjusted aaastatsd service restart in systemd service file. Fixes for py-swsssdk API changes. Only start aaastatsd for RADIUS statistics.
This pull request introduces 5 alerts when merging ac3e9a3 into 38f65c8 - view on LGTM.com new alerts:
|
- LGTM fixes
regression introduced in sonic-net#7284 Signed-off-by: Guohan Lu <[email protected]>
regression introduced in #7284 Signed-off-by: Guohan Lu <[email protected]>
Why I did it HLD: https://github.com/Azure/SONiC/blob/master/doc/aaa/radius_authentication.md CLI: In a separate PR. How I did it How to verify it UT: src/sonic-host-services/tests/hostcfgd/hostcfgd_radius_test.py
regression introduced in sonic-net#7284 Signed-off-by: Guohan Lu <[email protected]>
Why I did it HLD: https://github.com/Azure/SONiC/blob/master/doc/aaa/radius_authentication.md CLI: In a separate PR. How I did it How to verify it UT: src/sonic-host-services/tests/hostcfgd/hostcfgd_radius_test.py
regression introduced in sonic-net#7284 Signed-off-by: Guohan Lu <[email protected]>
os.unlink(stats_file) | ||
else: | ||
open(stats_file, 'a').close() | ||
os.chmod(stats_file, 0o666) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Semgrep detects a widely permissive file permission issue.
scripts/aaastatsd
python.lang.security.audit.insecure-file-permissions.insecure-file-permissions
These permissions `0o666` are widely permissive and grant access to more people than may be
necessary. A good default is `0o644` which gives read and write access to yourself and read
access to everyone else.
Details: https://sg.run/AXY4
154┆ os.chmod(stats_file, 0o666)
Github code scanning also detects same issue.
Is it possible to limit the rw access to the owner only and give no access to others? #Closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@maipbui
I think 0o644 or 0o600 might be ok. There is no sensitive information in this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @a-barboza! I’ll raise a PR to address this.
Why I did it
HLD: https://github.com/Azure/SONiC/blob/master/doc/aaa/radius_authentication.md
CLI: In a separate PR.
How I did it
How to verify it
UT: src/sonic-host-services/tests/hostcfgd/hostcfgd_radius_test.py
Which release branch to backport (provide reason below if selected)
Description for the changelog
A picture of a cute animal (not mandatory but encouraged)