RADIUS Management User Authentication Feature#7284
Merged
renukamanavalan merged 5 commits intosonic-net:masterfrom Apr 24, 2021
a-barboza:master
Merged
RADIUS Management User Authentication Feature#7284renukamanavalan merged 5 commits intosonic-net:masterfrom a-barboza:master
renukamanavalan merged 5 commits intosonic-net:masterfrom
a-barboza:master
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
Contributor
Author
|
/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
renukamanavalan
approved these changes
Apr 24, 2021
lguohan
added a commit
to lguohan/sonic-buildimage
that referenced
this pull request
Apr 24, 2021
regression introduced in sonic-net#7284 Signed-off-by: Guohan Lu <lguohan@gmail.com>
4 tasks
lguohan
added a commit
that referenced
this pull request
Apr 25, 2021
regression introduced in #7284 Signed-off-by: Guohan Lu <lguohan@gmail.com>
raphaelt-nvidia
pushed a commit
to raphaelt-nvidia/sonic-buildimage
that referenced
this pull request
May 23, 2021
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
raphaelt-nvidia
pushed a commit
to raphaelt-nvidia/sonic-buildimage
that referenced
this pull request
May 23, 2021
regression introduced in sonic-net#7284 Signed-off-by: Guohan Lu <lguohan@gmail.com>
carl-nokia
pushed a commit
to carl-nokia/sonic-buildimage
that referenced
this pull request
Aug 7, 2021
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
carl-nokia
pushed a commit
to carl-nokia/sonic-buildimage
that referenced
this pull request
Aug 7, 2021
regression introduced in sonic-net#7284 Signed-off-by: Guohan Lu <lguohan@gmail.com>
maipbui
reviewed
Jan 13, 2023
| os.unlink(stats_file) | ||
| else: | ||
| open(stats_file, 'a').close() | ||
| os.chmod(stats_file, 0o666) |
Contributor
There was a problem hiding this comment.
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
Contributor
Author
There was a problem hiding this comment.
@maipbui
I think 0o644 or 0o600 might be ok. There is no sensitive information in this file.
Contributor
There was a problem hiding this comment.
Thanks @a-barboza! I’ll raise a PR to address this.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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)