[Home]

Summary:ASTERISK-10653: [branch] Implement asterisk CLI permissions.
Reporter:Eliel Sardanons (eliel)Labels:
Date Opened:2007-10-30 13:50:08Date Closed:2008-12-01 12:52:13.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/NewFeature
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) cli.permissions.group.patch12
Description:Restrict users to run only a subset of commands allow (configured by an administrator).
You need write access to the asterisk.ctl socket file.
This is useful when you need to allow run commands on the asterisk CLI to some users for support purposes also is a secure manner to prevent commands like 'restart now' or 'stop now' being executed by mistake.

****** ADDITIONAL INFORMATION ******

The patch has a permissions.conf file added to the svn tree, that is used to configure the users permissions. The syntax is simple enough and allow the definition of templates that can be share among different users.
If we don't have the permissions.conf asterisk remains as it is now.
I also notice that rasterisk (asterisk -r) executes three CLI commands on startup:
'core set verbose atleast 0'
'core set debug atleast 0'
'logger mute silent'
If they are not allowed on the permissions.conf file they will not be executed.
I recommend adding them on a base template (example):
[rasterisk](!)
allow=core set verbose atleast 0
allow=core set debug atleast 0
allow=logger mute silent

For further documentation please refer to the permissions.conf on the uploaded patch file.
Comments:By: Jason Parker (jparker) 2007-10-30 18:14:07

You might want to check how ! commands work when logging in as a different user.

If commands are executed as the user that asterisk is running as, something like `!asterisk -rx "restart now"` would defeat this entire permissions stuff (note that ! isn't treated the same as a CLI command).
By: Eliel Sardanons (eliel) 2007-10-31 08:01:20

Login name: eliel
The 'cli permissions check <username>' command shows all the allowed commands for this user. As you could see user 'eliel' does not has permissions to run command 'stop now'. (Asterisk is running as root uid=0)

eliel*CLI> cli permissions check eliel
        cli permissions check Try a permissions config for a user
       cli permissions reload Reload CLI permissions config
         cli permissions show Show CLI permissions
       core set debug channel Enable/disable debugging on a channel
core set {debug|verbose} [off| Set level of debug/verbose chattiness
              core set global Set global dialplan variable
                sip show peer Show details on specific SIP peer
eliel*CLI> ! asterisk -rx "stop now"
You don't have permissions to run 'stop now' command
eliel*CLI> ! id
uid=1000(eliel) gid=1000(eliel) groups=4(adm),20(dialout),24(cdrom),25(floppy),29(audio),30(dip),44(video),46(plugdev),104(scanner),108(lpadmin),109(admin),115(netdev),117(powerdev),1000(eliel)

Then, I have enable command 'stop now' for user 'eliel' on the permissions.conf file and:

eliel*CLI> cli permissions check eliel
        cli permissions check Try a permissions config for a user
       cli permissions reload Reload CLI permissions config
         cli permissions show Show CLI permissions
       core set debug channel Enable/disable debugging on a channel
core set {debug|verbose} [off| Set level of debug/verbose chattiness
              core set global Set global dialplan variable
                module reload Reload configuration
                sip show peer Show details on specific SIP peer
                     stop now Shut down Asterisk immediately
eliel*CLI> ! asterisk -rx "stop now"
eliel*CLI>
Disconnected from Asterisk server
By: Jason Parker (jparker) 2007-10-31 10:01:57

perfect
By: Brandon Kruse (bkruse) 2007-10-31 11:52:11

This is awesome, I really like this.

Good job :]

One step closer to making it multi-user for multiple administrators.

-bk


By: Eliel Sardanons (eliel) 2007-10-31 14:49:00

New patch added against revision r87889
This new patch solves a problem while doing a 'asterisk -rx "command"' the problem was that my read_credentials() sometimes was reading the 'command' as the CREDENTIALS message and it was not executed. Now it is solved because only 12 bytes will be read by read_credentials().
Thanks to qwell for tell me to test that command and sorry for the bug (upsss).
By: Eliel Sardanons (eliel) 2007-10-31 14:52:16

bkruse: which are the other "steps" that needs to be done to support multi-user, any idea?
By: Eliel Sardanons (eliel) 2007-10-31 14:55:56

As you can see i have done three cli commands:
'cli permissions show'
      Will reply with the configuration loaded for each user.
'cli permissions reload'
       Reload the configuration file.
'cli permissions check <username> [<command>]'
       If a command is specified then an output message telling if user (<username>) is allow to run command ([<command>]). will be display
       If no command is specified all the allowed commands for user (<username>) will be shown.

Do you think I could added another CLI command?


By: Eliel Sardanons (eliel) 2007-10-31 22:09:23

cli.permissions.patch3 uploaded against r87971
+ Added missing 'ast_config_destroy(cfg)'
+ Added config_flag 'CONFIG_FLAG_FILEUNCHANGED' to ast_config_load() to check if the file changed.
+ Added more doxy comments.
By: Eliel Sardanons (eliel) 2007-11-19 16:47:35.000-0600

cli.permissions.patch4 Added to fixed a conflict while patching .patch3 to the latest revision.
Also being tested on last revision without problems.
By: Eliel Sardanons (eliel) 2007-11-19 16:50:10.000-0600

cli.permissions_with_sampleconfig.patch4 added.
Sorry, forgot to include permissions.conf on the svn diff.
By: Eliel Sardanons (eliel) 2007-11-24 14:31:42.000-0600

File added cli.permissions.patch5
- Fix conflicts with last revision
By: Eliel Sardanons (eliel) 2007-11-26 10:30:19.000-0600

It would be helpfull if someone could test this patch and give feedback, I have tested it, but one person is not enough.

Thanks in advanced
By: Igor Goncharovsky (igorg) 2007-11-26 23:27:09.000-0600

Ok, I'll try this and report results.
By: Tomás Laureano Peralta Tormey (laureano) 2007-11-27 09:00:37.000-0600

I has been testing this patch and is working like a charm in my development box.
The only suggestion that I have to make is to change "You are not allow to run any command on Asterisk" by "You are not allow_ed_ to run any command on Asterisk".

Thank you for this patch.
By: Eliel Sardanons (eliel) 2007-11-27 11:50:36.000-0600

Thanks Laureano, I will replace that string.
By: Eliel Sardanons (eliel) 2007-11-27 15:34:29.000-0600

cli.permissions.patch6 added against revision 89772 and a minor typo fix.
By: Eliel Sardanons (eliel) 2007-11-27 15:39:08.000-0600

Thanks IgorG I will wait your feedback
By: Igor Goncharovsky (igorg) 2007-11-27 23:55:41.000-0600

Impressive work, eliel! I have tested for a while and have following suggestions:

- After I have entered under user I have message: "You don't have permissions to run 'logger mute silent' command". ?his command automatically and must be run even we have not permission on it, I think
- On Tab auto completion I have list of all commands, I think here must be only commands allowed to user
- Also we have special command "help" which I think must be auto allowed for getting help on allowed command

That's all for now. Thank you again.

PS. Also, is it possible in same way secure cli command calling from manager interface?


By: Eliel Sardanons (eliel) 2007-11-28 11:41:21.000-0600

IgorG: Thanks for your feedback.

- That command is being run automatically while connecting with rasterisk, I didn't want to hardcode any value, I think allowing it on every user is better done harcoding this command to be allowed. But this is my idea, I think other asterisk-devs could tell me if it is better to hardcode that command to be allowed or leave it like it is now.

- I already though about limiting the autocompletion, the problem with that is that in many CLI commands autocompletion is handled inside the CLI command handler not in the ->command = "sip show peer" so to implement this we will need to pass the uid to the CLI command handler and check the permissions in the CLI command handler that is not very clean, and could bring many "security" bugs if it not well checked/done.

- I have already done a patch to cleanup the help command with the allowed command only, but is the same problem as the autocompletion, the 'help' command is another CLI command, so to check security I will need to pass the uid to the CLI handler and is a design change.

But what I have done to resolve what you tell me was, pass the uid of the user that is running the command in the ast_cli_args structure and the we have the uid in a->uid, so we can check security in autocompletion and in the help command, but I think this is a big design change and should be approved by the comunity, I think it is very difficult to start checking the security in every CLI command without adding new bugs to the core.

Thanks again for the feedback, and if you have an idea of how we could implement in a clean way those changes please let me know!

Eliel
By: Eliel Sardanons (eliel) 2007-11-28 14:21:57.000-0600

IgorG: and about your question if this patch could be used while running cli commands from manager the answer is yes, if the manager username is a real local unix user (if not, then there will be no permissions applied so default_perm will be used). But, right now i didn't want to do such integration, it is only needed to change NO_PERMS to the userid logged in on the manager interface.
By: Eliel Sardanons (eliel) 2007-11-29 10:02:27.000-0600

Added cli.permissions.patch7 file to fix some conflicts with latest revision.
By: Eliel Sardanons (eliel) 2007-12-06 10:35:04.000-0600

Added cli.permissions.patch8 against latest trunk to fix a minor conflict.
By: Tzafrir Cohen (tzafrir) 2007-12-06 15:41:08.000-0600

There is "allow=all" and "allow="name of command". What other special values do we want to reserve for special usage?

e.g: "disallow=write" or "allow=read" ?
By: Eliel Sardanons (eliel) 2007-12-06 19:14:23.000-0600

On the [general] you have default_perm=allow|disallow

allow=all
disallow=all
allow|disallow="command name"

If command name is incomplete will match all the other subsequent commands like:
allow=sip   <-- all commands starting with sip will be allowed (sip show peers, sip show peer, etc).
The configuration is simple like this.

I think we could added the possibility to allow or disallow permissions to a group:

[@groupname]
disallow=all
allow=core set verbose

Will disallow all cli commands for users in the group 'groupname', if the user has a specific 'context' will overwrite the group config.
By: Eliel Sardanons (eliel) 2007-12-06 23:39:13.000-0600

Added new patch cli.permissions.group.patch (r91700), now we have support for local groups too.
Groups are defined like users but with a '@' at the beginning.
Groups permissions are overwritten by users permissions.
By: Eliel Sardanons (eliel) 2007-12-22 13:00:32.000-0600

Upload new patch: cli.permissions.group.patch9 against latest trunk (r94593).

.permissions.patchX (supports only permissions applied to the user).
.permissions.group.patchX (supports permissions being applied to the group and the user).
By: Jason Parker (jparker) 2008-01-17 15:55:25.000-0600

In cli_has_permissions, you are missing a lock in the uid case.  You can probably also combine those loops..

I would argue for permit/deny (or similar?), rather than allow/disallow.  permit/deny are already used for ACL types of things in other places, whereas allow/disallow is used for codecs.  It could be a little confusing.

Should user perms "overwrite" group perms?  I think if both a user and group are specified, that the user should get all of the commands in @group, as well as any additional things in his user (it could get tricky if you have [@group]disallow=all,allow=foo and [user]disallow=all,allow=bar).  I suspect it would end up being quite similar to templates.

I'm also concerned whether or not this will work on non-Linux machines.
By: Eliel Sardanons (eliel) 2008-01-17 16:17:34.000-0600

Thanks qwell for the feedback, I will review my code. It is supposed to work on non-Linux machines, at least a discussion on asterisk-dev about using this authentication method was about the compatibility and we arrive to the answer that it should work, but obviusly more testing is needed.
About the group settings being overwritten by the user settings, I think is the best way (I know that there are many permissions configurations that could bring confusion to the end user), but we could listen other opinions and if a change is needed I will doit :-). I would like to know also, how do you think those permissions should be checked, you think that if a permit=all or a deny=all has been already setted by the group then a permit=all or a deny=all configured on the user should be discarded?

Thanks in advanced


By: Jason Parker (jparker) 2008-01-17 16:19:32.000-0600

I was thinking about that case, and I don't think any options should be ignored.  I think it should just be up to the implementor to make sure things like that don't happen.  The same would be true with a template (your example has a template, and you left out disallow=all for that very reason)
By: Eliel Sardanons (eliel) 2008-01-17 16:26:08.000-0600

Ok!, I will come up with the suggested mods and the fixing as soon as I can, to let other test it on non linux enviroments.
By: Eliel Sardanons (eliel) 2008-01-17 16:27:38.000-0600

We should notice that if the credentials are not being passed throw the socket, then this feature won't be enabled and the asterisk cli will continue working as normal without permissions.
By: Eliel Sardanons (eliel) 2008-05-27 14:33:30

Added cli.permissions.group.patch10
- Update patch against SVN trunk.
- Changed allow to permit and disallow to deny in the config file.

* I couldn't join the two loops in cli_has_permissions cause user permissions overwrite group permissions, and I couldn´t find a way to join the loops thinking in this scenario.
By: Eliel Sardanons (eliel) 2008-06-20 10:25:11

What I can do to continue moving this patch? Any feedback?

Thanks in advanced
By: Eliel Sardanons (eliel) 2008-07-13 20:55:31

cli.permissions.group.patch11 uploaded, patch against revision (130632).
One change was added, now we don't send an special message to send the credentials (ugly!), we send the credentials in every command and we receive the credentials in every command.
By: Eliel Sardanons (eliel) 2008-07-13 20:56:24

for this purpose the main/asterisk.c/read_credentials() is a wrapper for the read() syscall.
By: Eliel Sardanons (eliel) 2008-07-25 14:46:10

New patch uploaded, features:
- CLI command 'cli permissions show' now shows the name of the user/group on the output.
- If no credentials are being passed then the default_perm configured in permissions.conf will be applied.
- Added more documentation.
By: Eliel Sardanons (eliel) 2008-08-02 02:09:05

A branch was created to continue development there:
http://svn.digium.com/svn/asterisk/team/eliel/cli-permissions
By: dovid (dovid) 2008-10-07 13:21:01

Can some one post info here for the layman on how to set up permissions.conf and once I am in the CLI on how to work it ?
By: Eliel Sardanons (eliel) 2008-10-07 13:26:12

permissions.conf is of the form:

[username]
permit=command
permit=command
deny=commad

where command is part or the complete CLI command, like "sip show", "sip" ,etc. and also the hardcoded value "all" to represent every command (permit=all or deny=all).
By: Leif Madsen (lmadsen) 2008-10-22 10:49:05

There is a small conflict that eliel needs to resolve because of the appxmldocs stuff he has been working on, so I'm changing this back to Confirmed for now. Eliel will change back to Ready for Testing when the conflicts have been resolved. Thanks for the update!
By: Leif Madsen (lmadsen) 2008-10-24 08:55:02

Conflict resolved.
By: Terry Wilson (twilson) 2008-10-27 14:02:11

I tested and found a crash if you ran rasterisk and it tried to execute commands that you didn't have permission for.  Fixed the crash and committed.  Tested and found no other issues.  I think this should be good to merge.
By: Terry Wilson (twilson) 2008-11-24 15:55:59.000-0600

I have updated the branch to fix a couple of issues that popped up since the inclusion of the clialiases stuff.  It looks good to go again, now.  Whenever eliel wants to merge it is fine with me, anyway...
By: Digium Subversion (svnbot) 2008-12-01 12:52:10.000-0600

Repository: asterisk
Revision: 160062

U   trunk/CHANGES
A   trunk/configs/cli_permissions.conf.sample
U   trunk/configure
U   trunk/configure.ac
U   trunk/include/asterisk/_private.h
U   trunk/include/asterisk/autoconfig.h.in
U   trunk/include/asterisk/cli.h
U   trunk/main/asterisk.c
U   trunk/main/cli.c

------------------------------------------------------------------------
r160062 | eliel | 2008-12-01 12:52:09 -0600 (Mon, 01 Dec 2008) | 13 lines

Introduce CLI permissions.
Based on cli_permissions.conf configuration file, we are able to permit or deny
cli commands based on some patterns and the local user and group running rasterisk.

(Sorry if I missed some of the testers).

Reviewboard: http://reviewboard.digium.com/r/11/

(closes issue ASTERISK-10653)
Reported by: eliel
Tested by: eliel, IgorG, Laureano, otherwiseguy, mvanbaak


------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=160062