-
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
[tacacs] Support privilege level 0 #1442
base: master
Are you sure you want to change the base?
Conversation
@liuqu, could you kindly help review? |
@@ -463,7 +463,7 @@ index 79e62b9..ecfa0b0 100644 | |||
+{ | |||
+ char *token; | |||
+ char delim[] = ";\n\r"; | |||
+ int priv = 0; | |||
+ int priv = -1; |
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.
The default value of “priv_level” in function got_tacacs_user() also need to be changed
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.
TACACS priv-level 0 is seldom used and defined as non-privilege in general. Any scenarios need to support it?
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.
does non-privilege mean normal user?
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.
privilege level 0 = seldom used, but includes 5 commands: disable, enable, exit, help, and logout
it is still valid, we should still enable it.
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.
Why do we need to modify default value in got_tacacs_user()? I think current behavior is that to treat user as least privileged (0) if search fails?
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.
The original design is that the priv-lvl for the user configuration on the TACACS + server should be set as a valid value(1-15) explicitly. If priv-lvl search fails, it will be set as privileged(0) and not allowed to login in.
If priv-lvl(0) is treated as valid value, the default value should be set as invalid value. Unless we don't care priv-lvl.
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.
I think non-privilege means the user has less command privilege. There is no authorization for current SONiC cli, so the original design is to filter non-privilege user to avoid unsafe operation. Maybe it's too hard-coded to limit user's configuration.
@taoyl-ms , do we still need this? |
Avoid adding loopback interface (ip link add) when setting nat zone on loopback interface (#1434) [acl] Remove Ethertype from L3V6 qualifiers (#1433) Sflow fixes during DEL processing (#1427) Fix #3971 by skipping create-only SAI attributes when modifying buffer pools or profiles in orchagent (#1430) Fix issue: bufferorch only pass the first attribute to sai when setting attribute (#1442) Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
be51ebc Add IPv6 key item support to request parser (sonic-net#1449) 76e2251 When teamd feature state is disabled the Netdevice created by teamd were (sonic-net#1450) 6aa97ce Use .clear() after std::move() (sonic-net#1444) d5757db Add libzmq to README dependencies (sonic-net#1447) c7b262e Add libzmq to Makefiles (sonic-net#1443) 0b2e59a [drop counters] Clarify log messages for initial counter setup (sonic-net#1445) 003cf24 [dvs] Refactor and add buffer pool wm test (sonic-net#1446) 2f5d2d9 [acl] Remove Ethertype from L3V6 qualifiers (sonic-net#1433) f7b974f Fix issue: bufferorch only pass the first attribute to sai when setting attribute (sonic-net#1442) Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
be51ebc Add IPv6 key item support to request parser (#1449) 76e2251 When teamd feature state is disabled the Netdevice created by teamd were (#1450) 6aa97ce Use .clear() after std::move() (#1444) d5757db Add libzmq to README dependencies (#1447) c7b262e Add libzmq to Makefiles (#1443) 0b2e59a [drop counters] Clarify log messages for initial counter setup (#1445) 003cf24 [dvs] Refactor and add buffer pool wm test (#1446) 2f5d2d9 [acl] Remove Ethertype from L3V6 qualifiers (#1433) f7b974f Fix issue: bufferorch only pass the first attribute to sai when setting attribute (#1442) Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
…ng attribute (sonic-net#1442) Signed-off-by: Stephen Sun <stephens@nvidia.com>
be51ebc Add IPv6 key item support to request parser (sonic-net#1449) 76e2251 When teamd feature state is disabled the Netdevice created by teamd were (sonic-net#1450) 6aa97ce Use .clear() after std::move() (sonic-net#1444) d5757db Add libzmq to README dependencies (sonic-net#1447) c7b262e Add libzmq to Makefiles (sonic-net#1443) 0b2e59a [drop counters] Clarify log messages for initial counter setup (sonic-net#1445) 003cf24 [dvs] Refactor and add buffer pool wm test (sonic-net#1446) 2f5d2d9 [acl] Remove Ethertype from L3V6 qualifiers (sonic-net#1433) f7b974f Fix issue: bufferorch only pass the first attribute to sai when setting attribute (sonic-net#1442) Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
…ng attribute (sonic-net#1442) Signed-off-by: Stephen Sun <stephens@nvidia.com>
…tically (#20540) #### Why I did it src/sonic-sairedis ``` * e394ced7 - (HEAD -> master, origin/master, origin/HEAD) Fix compilation on Buster (#1449) (11 hours ago) [Saikrishna Arcot] * 4d504ff8 - Rename file name to fit case insensitive file system. (#1444) (2 days ago) [Liu Shilong] * fe650bb7 - [syncd] Add workaround for port error status notification (#1430) (6 days ago) [Kamil Cudnik] * cd2773a3 - [syncd] Fix inspect asic command (#1434) (7 days ago) [Kamil Cudnik] * 2d873766 - [syncd] Make sure notification queue release memory when drained (#1427) (8 days ago) [Kamil Cudnik] * b8a8856a - Fix adding flex counter to wrong context (#1421) (8 days ago) [byu343] * 40979e0b - [fastboot] Notify SAI that fastboot is done (#1396) (8 days ago) [Junchao-Mellanox] * 952ee406 - [codeql] Change pull_request_target to pull_request (#1442) (9 days ago) [Kamil Cudnik] * 697d86b5 - [syncd] Create neighbor entries before next hop (#1432) (9 days ago) [Kamil Cudnik] * fa76ca13 - [codeql] Remove git ancestry (#1441) (10 days ago) [Kamil Cudnik] * 3838d7ee - [codeql] Show git ancestry graph (#1440) (10 days ago) [Kamil Cudnik] * 2e7d946b - [codeql] Show gcc version before compile (#1438) (10 days ago) [Kamil Cudnik] * a1e93f58 - [submodule] Update SAI to latest master (#1431) (2 weeks ago) [Kamil Cudnik] ``` #### How I did it #### How to verify it #### Description for the changelog
- What I did
To support tacacs user privilege level 0 by setting
MIN_TACACS_USER_PRIV = 0
in tacacs-nss patch.