-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add SONiC design doc: TACACS+ improvement #813
Conversation
liuh-80
commented
Jul 9, 2021
•
edited
Loading
edited
Repo | PR title | State |
---|---|---|
sonic-buildimage | Add plugin support to bash | |
sonic-buildimage | Extract tacacs support functions into library and fix memory leak issue. | |
sonic-buildimage | Add Bash TACACS+ plugin for per-command authorization. | |
sonic-buildimage | Add Config DB schema and HostCfg Enforcer plugin to support TACACS+ per-command authorization&accounting. | |
sonic-utilities | Add config command for AAA authorization and accounting. | |
sonic-mgmt | Add TACACS per-command authorization test case. | |
sonic-buildimage | Add audisp-tacplus for per-command accounting. | |
sonic-mgmt | Add TACACS per-command accounting test case. | |
sonic-buildimage | Fix bash build break when re-build bash issue. | |
sonic-buildimage | fix src\tacacs\bash_tacplus\debian\rules file mode, which cause dirty image version. | |
sonic-buildimage | Export remote address to environment variable for TACACS authorization. | |
sonic-buildimage | Send remote address in TACACS+ authorization message. | |
sonic-buildimage | Fix per-command authorization failed issue when a command with wildcard match more than hundred files. | |
sonic-buildimage | Fix auditd can't load tacplus plugin issue. |
doc/aaa/TACACS+ Requirement.md
Outdated
- Different privilege level have different permission to run these command. | ||
- All commands in sudoers will add to the whitelist. and sudoers config file still need for RO users, this is because when remote TACACS server not avaliable, we need use local group permission for failover. | ||
|
||
- Disable user behavior in shell: |
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 rbash is a must for command authorization?
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 reason is because for command authorization, we create a symbol link for every command we want enable authorization.
Without rbash user can run any command, include those command we not create symbol link.
Then they can bypass the authorization very easy.
doc/aaa/TACACS+ Requirement.md
Outdated
- Specifying command names containing slashes. | ||
- Importing function definitions from the shell environment at startup. | ||
- Parsing the value of SHELLOPTS from the shell environment at startup. | ||
- Redirecting output using the ‘>’, ‘>|’, ‘<>’, ‘>&’, ‘&>’, and ‘>>’ redirection operators. |
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.
By patch bash, we can allow user use redirect, will remove this from document.
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.
Do you have a link to the patch? I am wondering if redirect could be configurable. For example, RW users could use redirect as normal, but RO users are banned to use redirect.
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.
It's a very simple code change, remove following code in redir.c line:836
#if defined (RESTRICTED_SHELL)
if (restricted && (WRITE_REDIRECT (ri)))
{
free (redirectee_word);
return (RESTRICTED_REDIRECT);
}
#endif /* RESTRICTED_SHELL */
So for RW allow and RO disable, we can:
-
create a variable isReadonlyUser
-
When bash startup, check current user group and set this variable.
-
If any behavior we want to disable for RO only we can change code like this:
#if defined (RESTRICTED_SHELL)
if (restricted && isReadonlyUser && (WRITE_REDIRECT (ri)))
{
free (redirectee_word);
return (RESTRICTED_REDIRECT);
}
#endif /* RESTRICTED_SHELL */
doc/aaa/TACACS+ Requirement.md
Outdated
- Different privilege level have different permission to run these command. | ||
- All commands in sudoers will add to the whitelist. and sudoers config file still need for RO users, this is because when remote TACACS server not avaliable, we need use local group permission for failover. | ||
|
||
- Disable user behavior in shell: |
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.
In that case, we need consider some other solution. I will check other solution in POC and update this document.
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 check some other solution and they are based on shell function hook, those solution have a common limitation: user can bypass restriction by ingest new shell hook.
Then after check the rbash restriction, I think we have 2 solution:
-
Patch bash to allow redirection in restrict mode, and accept most other restriction because those are necessary for use tacplus-auth: those restriction we accept will make both RO/RW user can't access commands not in our white list, and make user can't switch to other shell.
-
Patch bash, so bash can do Authorization before execute any user command.
With this solution, there is no any restriction in router machine side, in TACACS server side we just need disable user switch to other shell.
I will try a POC for solution 2, it's seems will be a minor change.
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 POC for solution 2 is ready, which will authorize any user 'disk' command, so if solution 2 is acceptable, will update the document.
For 'disk' command, in bash, if a command is a file stored in disk, it's a disk command. built-in command and bash function are not disk command, executable file and script are disk command.
doc/aaa/TACACS+ Requirement.md
Outdated
- Counter command | ||
``` | ||
show tacacs counter | ||
clear tacacs counter |
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.
Fixed.
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.
We have a similar command sonic-clear
. ref: https://github.com/Azure/sonic-utilities/blob/888701b67fd4f1cc5b9da534a360048f93f263f4/setup.py#L157
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.
Fixed with sonic-clear command, will push update later.
doc/aaa/TACACS+ Requirement.md
Outdated
|
||
## 2.4 System log | ||
- Generate system log when Authentication/Authorization/Accounting. | ||
- When remote TACACS server not avaliable, use system log for accounting. |
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.
I think it's not necessary.
If tacacs server is accessible, we only put trace log to syslog for metering and debugging.
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 guess your system log
== syslog
.
Then if the networking is not stable, you will have some random log lines locally, does it help anything?
For login authentication, the log messages always go to local syslog, no matter remote tacacs accessible or not.
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.
Document updated.
What I mean here is when the remote TACACS service not accessible, for example TACACS server down after user login, but networking still healthy, then the Authorization and Accounting information will write to syslog. Then the syslog will upload via network later, and those information will not lost.
doc/aaa/TACACS+ Design.md
Outdated
## 3.2 Server count | ||
- Max TACACS server count was hardcoded, default count is 8. | ||
|
||
## 3.1 Local authorization |
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.
Fixed.
doc/aaa/TACACS+ Design.md
Outdated
- Max TACACS server count was hardcoded, default count is 8. | ||
|
||
## 3.1 Local authorization | ||
- Operation system limitation: SONiC based on linux system, so permission to execute local command are managed by Linux file permission control. This means TACACS+ authorization can't config to disable 'local' authorization, and local authorization must be last authorization in authorization method list. |
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.
Fixed.
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.
My point is that you have 2 options
- tacacs + local
- tacacs only, no local
And this is not a "Operation system limitation".
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.
Here is a example of this issue:
user name: domainuser
When user login, tacacs return privilege level 0 so in SONiC this user is a RO user, then this user on sonic only have Linux permission to run RO commands.
But in tacacs server side, the 'config' command was config as allow user run.
Then when user trying to run the 'config' command, tacacs authorization succeeded, but user still can't run this command because user not have Linux permission.
In EOS system, the behavior is different:
We add domainuser as local RO user.
In tacacs server config this user can run RW command.
Then we user run a RW command, user can run it.
The reason is the EOS system can bypass the local permission check, but SONiC is a Linux system, and currently we using Linux permission for 'local authorization', so tacacs authorization can't bypass the Linux permission check.
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 see your point!
Actually I am considering another config "tacacs only, no fallback to local", which means tacacs server unreachable, will stop the already-login user to run anything except logout
or exit
. Then the user need to re-login with local account, local authentication and local authorization.
I wonder if SELinux would be better suited to restrict user commands? Here is some examples: https://access.redhat.com/solutions/65822 |
@seiferteric, Thanks for your comments, about your concern:
I verify following cases with my POC code, and command can be blocked with TACACS+ server side setting:
For the LD_PRELOAD issue, current solution can't block it because this not a command parameter. However this not be a issue because: |
doc/aaa/TACACS+ Design.md
Outdated
## 2.1 SONiC CLI | ||
- Enable/Disable TACACS Authorization/Accounting command | ||
``` | ||
config aaa authorization {local | tacacs+} |
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.
Will change to config aaa authorization {local | tacacs+ | local tacacs+}
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.
Fixed.
Verify TACACS+ user can't run command not in server side whitelist. | ||
``` | ||
|
||
- config AAA authorization with TACACS+ only: |
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.
This a very practical work flow. Normally users found tacacs user could run nothing, they will logout and login with local account, and run command as local authorization, and even tacacs server recovered then, it does not impact local user command authorization. #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.
config AAA authorization with TACACS+ and local, but server not accessible:
Add this step: when remote tacacs accessible, local user still can run command. doesn't impact local user authorization.
Check if we have PR cover this: when tacacs accessible, local can't login.
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.
Discuss offline, and you will add some step in another test case below.
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.
Fixed.
doc/aaa/TACACS+ Design.md
Outdated
Verify TACACS+ user can run command not in server side whitelist but have permission in local. | ||
Verify TACACS+ user can't run command in server side whitelist but not have permission in local. | ||
Verify Local user can login, and run command with local permission. | ||
Verify after Local user login, then server accessible, Local user still can run command with local permission. |
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.
Fixed. also update AAA table schema according to current SONiC yang model.
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.
Only minor word issue.
Please link the code PR by referring to this example #806 |