Adding another new filed categories at the end of
command reply, it's easy to read and distinguish
flags and categories, also compatible with old format.
That's not REALLY needed, but... right now with LASTSAVE being the only
command tagged as "admin" but not "dangerous" what happens is that after
rewrites the rewrite engine will produce from the rules:
user default on +@all ~* -@dangerous nopass
The rewrite:
user default on nopass ~* +@all -@admin -@dangerous +lastsave
Which is correct but will have users wondering about why LASTSAVE has
something special.
Since LASTSAVE after all also leaks information about the underlying
server configuration, that may not be great for SAAS vendors, let's tag
it as dangerous as well and forget about this issue :-)
In some cases processMultibulkBuffer uses sdsMakeRoomFor to
expand the querybuf, but later in some cases it uses that query
buffer as is for an argv element (see "Optimization"), which means
that the sds in argv may have a lot of wasted space, and then in case
modules keep that argv RedisString inside their data structure, this
space waste will remain for long (until restarted from rdb).
In mostly production environment, normal user's behavior should be
limited.
Now in redis ACL mechanism we can do it like that:
user default on +@all ~* -@dangerous nopass
user admin on +@all ~* >someSeriousPassword
Then the default normal user can not execute dangerous commands like
FLUSHALL/KEYS.
But some admin commands are in dangerous category too like PSYNC,
and the configurations above will forbid replica from sync with master.
Finally I think we could add a new configuration for replication,
it is masteruser option, like this:
masteruser admin
masterauth someSeriousPassword
Then replica will try AUTH admin someSeriousPassword and get privilege
to execute PSYNC. If masteruser is NULL, replica would AUTH with only
masterauth like before.
__x86_64__ is defined on the on the x32 architecture and the conditionals in
debug.c therefore assume the size of (void*) etc:
debug.c: In function 'getMcontextEip':
debug.c:757:12: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
return (void*) uc->uc_mcontext.gregs[16]; /* Linux 64 */
^
debug.c: In function 'logRegisters':
debug.c:920:21: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
logStackContent((void**)uc->uc_mcontext.gregs[15]);
We can remedy this by checking for __ILP32__ first. See:
https://wiki.debian.org/ArchitectureSpecificsMemo
... for more info.
Soon or later we may have code in freeClient() that may have to deal
with ACLs. Imagine for instance the command proposed multiple times (not
sure if this will ever be accepted but still...):
ONCLOSE DEL mykey
Accumulating commands to run when a client is disconnected. Now the
function is compatible with such use cases.
Related to #5829.
We can't trust modules commands flagging, so module commands must be
always explicitly added, with the exception of +@all that will include
everything. However something like +@readonly should not include command
from modules that may be potentially dangerous: our categories must be
safe and reliable and modules may not be like that.
It does not make much sense to limit what modules can do: the admin
should instead limit what module commnads an user may call. So
RedisModule_Call() and other module operations should be able to execute
everything they want: the limitation should be posed by the API exported
by the module itself.
