gvsafronov/futriix
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

make processCommand check publish channel permissions. (#8534)

Browse Source 
Add publish channel permissions check in processCommand.

processCommand didn't check publish channel permissions, so we can
queue a publish command in a transaction. But when exec the transaction,
it will fail with -NOPERM.

We also union keys/commands/channels permissions check togegher in
ACLCheckAllPerm. Remove pubsubCheckACLPermissionsOrReply in 
publishCommand/subscribeCommand/psubscribeCommand. Always 
check permissions in processCommand/execCommand/
luaRedisGenericCommand.
This commit is contained in:
Huang Zhw 2021-03-26 19:10:01 +08:00 committed by GitHub
parent db6655deb4
commit e138698e54
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 55 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
src/acl.c
View File

@ -1360,6 +1360,22 @@ int ACLCheckPubsubPerm(client *c, int idx, int count, int literal, int *idxptr)
}
/* Check whether the command is ready to be exceuted by ACLCheckCommandPerm.
* If check passes, then check whether pub/sub channels of the command is
* ready to be executed by ACLCheckPubsubPerm */
int ACLCheckAllPerm(client *c, int *idxptr) {
int acl_retval = ACLCheckCommandPerm(c,idxptr);
if (acl_retval != ACL_OK)
return acl_retval;
if (c->cmd->proc == publishCommand)
acl_retval = ACLCheckPubsubPerm(c,1,1,0,idxptr);
else if (c->cmd->proc == subscribeCommand)
acl_retval = ACLCheckPubsubPerm(c,1,c->argc-1,0,idxptr);
else if (c->cmd->proc == psubscribeCommand)
acl_retval = ACLCheckPubsubPerm(c,1,c->argc-1,1,idxptr);
return acl_retval;
}
/* =============================================================================
* ACL loading / saving functions
* ==========================================================================*/

7
src/multi.c
View File

@ -204,9 +204,7 @@ void execCommand(client *c) {
/* ACL permissions are also checked at the time of execution in case
* they were changed after the commands were queued. */
int acl_errpos;
int acl_retval = ACLCheckCommandPerm(c,&acl_errpos);
if (acl_retval == ACL_OK && c->cmd->proc == publishCommand)
acl_retval = ACLCheckPubsubPerm(c,1,1,0,&acl_errpos);
int acl_retval = ACLCheckAllPerm(c,&acl_errpos);
if (acl_retval != ACL_OK) {
char *reason;
switch (acl_retval) {
@ -217,7 +215,8 @@ void execCommand(client *c) {
reason = "no permission to touch the specified keys";
break;
case ACL_DENIED_CHANNEL:
reason = "no permission to publish to the specified channel";
reason = "no permission to access one of the channels used "
"as arguments";
break;
default:
reason = "no permission";

18
src/pubsub.c
View File

@ -331,21 +331,6 @@ int pubsubPublishMessage(robj *channel, robj *message) {
return receivers;
}
/* This wraps handling ACL channel permissions for the given client. */
int pubsubCheckACLPermissionsOrReply(client *c, int idx, int count, int literal) {
/* Check if the user can run the command according to the current
* ACLs. */
int acl_chanpos;
int acl_retval = ACLCheckPubsubPerm(c,idx,count,literal,&acl_chanpos);
if (acl_retval == ACL_DENIED_CHANNEL) {
addACLLogEntry(c,acl_retval,acl_chanpos,NULL);
addReplyError(c,
"-NOPERM this user has no permissions to access "
"one of the channels used as arguments");
}
return acl_retval;
}
/*-----------------------------------------------------------------------------
* Pubsub commands implementation
*----------------------------------------------------------------------------*/
@ -353,7 +338,6 @@ int pubsubCheckACLPermissionsOrReply(client *c, int idx, int count, int literal)
/* SUBSCRIBE channel [channel ...] */
void subscribeCommand(client *c) {
int j;
if (pubsubCheckACLPermissionsOrReply(c,1,c->argc-1,0) != ACL_OK) return;
if ((c->flags & CLIENT_DENY_BLOCKING) && !(c->flags & CLIENT_MULTI)) {
/**
* A client that has CLIENT_DENY_BLOCKING flag on
@ -387,7 +371,6 @@ void unsubscribeCommand(client *c) {
/* PSUBSCRIBE pattern [pattern ...] */
void psubscribeCommand(client *c) {
int j;
if (pubsubCheckACLPermissionsOrReply(c,1,c->argc-1,1) != ACL_OK) return;
if ((c->flags & CLIENT_DENY_BLOCKING) && !(c->flags & CLIENT_MULTI)) {
/**
* A client that has CLIENT_DENY_BLOCKING flag on
@ -420,7 +403,6 @@ void punsubscribeCommand(client *c) {
/* PUBLISH <channel> <message> */
void publishCommand(client *c) {
if (pubsubCheckACLPermissionsOrReply(c,1,1,0) != ACL_OK) return;
int receivers = pubsubPublishMessage(c->argv[1],c->argv[2]);
if (server.cluster_enabled)
clusterPropagatePublish(c->argv[1],c->argv[2]);

4
src/scripting.c
View File

@ -604,9 +604,7 @@ int luaRedisGenericCommand(lua_State *lua, int raise_error) {
/* Check the ACLs. */
int acl_errpos;
int acl_retval = ACLCheckCommandPerm(c,&acl_errpos);
if (acl_retval == ACL_OK && c->cmd->proc == publishCommand)
acl_retval = ACLCheckPubsubPerm(c,1,1,0,&acl_errpos);
int acl_retval = ACLCheckAllPerm(c,&acl_errpos);
if (acl_retval != ACL_OK) {
addACLLogEntry(c,acl_retval,acl_errpos,NULL);
switch (acl_retval) {

22
src/server.c
View File

@ -4011,18 +4011,30 @@ int processCommand(client *c) {
/* Check if the user can run this command according to the current
* ACLs. */
int acl_keypos;
int acl_retval = ACLCheckCommandPerm(c,&acl_keypos);
int acl_errpos;
int acl_retval = ACLCheckAllPerm(c,&acl_errpos);
if (acl_retval != ACL_OK) {
addACLLogEntry(c,acl_retval,acl_keypos,NULL);
if (acl_retval == ACL_DENIED_CMD)
addACLLogEntry(c,acl_retval,acl_errpos,NULL);
switch (acl_retval) {
case ACL_DENIED_CMD:
rejectCommandFormat(c,
"-NOPERM this user has no permissions to run "
"the '%s' command or its subcommand", c->cmd->name);
else
break;
case ACL_DENIED_KEY:
rejectCommandFormat(c,
"-NOPERM this user has no permissions to access "
"one of the keys used as arguments");
break;
case ACL_DENIED_CHANNEL:
rejectCommandFormat(c,
"-NOPERM this user has no permissions to access "
"one of the channels used as arguments");
break;
default:
rejectCommandFormat(c, "no permission");
break;
}
return C_OK;
}

5
src/server.h
View File

@ -2092,7 +2092,7 @@ int isMutuallyExclusiveChildType(int type);
extern rax *Users;
extern user *DefaultUser;
void ACLInit(void);
/* Return values for ACLCheckCommandPerm() and ACLCheckPubsubPerm(). */
/* Return values for ACLCheckAllPerm(). */
#define ACL_OK 0
#define ACL_DENIED_CMD 1
#define ACL_DENIED_KEY 2
@ -2103,8 +2103,7 @@ int ACLAuthenticateUser(client *c, robj *username, robj *password);
unsigned long ACLGetCommandID(const char *cmdname);
void ACLClearCommandID(void);
user *ACLGetUserByName(const char *name, size_t namelen);
int ACLCheckCommandPerm(client *c, int *keyidxptr);
int ACLCheckPubsubPerm(client *c, int idx, int count, int literal, int *idxptr);
int ACLCheckAllPerm(client *c, int *idxptr);
int ACLSetUser(user *u, const char *op, ssize_t oplen);
sds ACLDefaultUserFirstPassword(void);
uint64_t ACLGetCommandCategoryFlagByName(const char *name);

16
tests/unit/acl.tcl
View File

@ -113,6 +113,22 @@ start_server {tags {"acl"}} {
set e
} {*NOPERM*channel*}
test {In transaction queue publish/subscribe/psubscribe to unauthorized channel will fail} {
r ACL setuser psuser +multi +discard
r MULTI
catch {r PUBLISH notexits helloworld} e
r DISCARD
assert_match {*NOPERM*} $e
r MULTI
catch {r SUBSCRIBE notexits foo:1} e
r DISCARD
assert_match {*NOPERM*} $e
r MULTI
catch {r PSUBSCRIBE notexits:* bar:*} e
r DISCARD
assert_match {*NOPERM*} $e
}
test {It's possible to allow subscribing to a subset of channels} {
set rd [redis_deferring_client]
$rd AUTH psuser pspass