OpenSSL 3 escapes '/' and '+' in X509_NAME_oneline() output; OpenSSL 1.1
and WolfSSL don't. A REQUIRE ISSUER/SUBJECT grant from one library can
stop matching after switching to another.
Default comparison stays strcmp(). old_mode=X509_LENIENT_COMPARE opts
into falling back to an escape-aware compare, applied to whichever side
the currently-linked library's own escaping affects, at the cost of
reopening the ambiguity a crafted certificate could exploit to
impersonate another identity.
Adds regression tests against a real certificate with an ambiguous CN
Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>
Defer the host-privileged/host-blocked check for a PROXY-header-derived
address until after the client's SSL handshake completes, instead of
sending it immediately in clear text.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The "is it my own thread" check compared the login user name
(Security_context::user) and ignored the host, so u1@localhost could see
and kill threads of u1@'127.0.0.1'.
Compare the authenticated account instead:
* all comparisons are done in sctx->is_priv_user() now
* change user_matches() to priv_user_matches(), which uses is_priv_user()
* use it in KILL, KILL USER, SHOW PROCESSLIST, I_S.PROCESSLIST,
COM_PROCESS_INFO and SHOW EXPLAIN/ANALYZE FOR.
* all the remaining places use is_priv_user() directly instead
of doing strcmp: SHOW GRANTS, SHOW CREATE PROCEDURE, I_S.VIEWS,
the DEFINER clause, optimizer trace and change_security_context().
Assisted-by: Claude:claude-5-opus
In propagate_role_grants_action() (called by grant_reload() on startup
and FLUSH PRIVILEGES), role privilege merging uses a counter-based
bottom-up ordering: each role's counter tracks how many of its granted
sub-roles still need to be processed, and merge_role_privileges() merges
a role only once its counter reaches zero.
The bug: when an intermediate role such as org_role_1 (which inherits
only from a USAGE-only app_role_1) was merged with no privilege changes,
merge_role_privileges() returned 1 as an optimisation to stop upward
traversal. This prevented the traversal from ever reaching user_role_1
to decrement its counter. A second leaf traversal (from app_role_2)
could decrement user_role_1's counter only once, leaving it at 1
instead of 0, so user_role_1 was never merged and had no effective
privileges.
The "stop if nothing changed" optimisation is valid for incremental
propagate_role_grants() calls (after a single GRANT/REVOKE), where
every ancestor was already correctly merged. It is not valid for the
full-reload case, where the counters must be decremented by every leaf
traversal to guarantee correct bottom-up ordering.
Fix: add an initial_load flag to PRIVS_TO_MERGE. When set (only in
propagate_role_grants_action), the early-stop optimisation is disabled
so traversals always visit all ancestors.
Assisted-By: Claude:claude-4.6-sonnet
a90779098b changed the
interface to the json_valid to take a json_engine_t
so that it could be interupted and so that warnings
could become visible.
On merge from that 10.11 to 11.4 it was discovered
that the ColumnStore engine uses this interface.
Restored the original json_valid function exactly
how ColumnStore expects this and added renamed
the function with json_engine_t to be json_valid_engine.
This follows the same convention as in the commit
756fc6fd9a that was
for json_normalize, that ColumnStore also used.
Or max_statement_time limit.
Change the json_valid to take a json_engine_t argument.
Adjust the Item_json_valid to have a json_engine_t and
to report an error.
Note this means that invalid json syntax now has a
note on any json error. Raising this to a warning can
cause check constraints of JSON_VALID() to result
in ER_JSON_SYNTAX rather than ER_CONSTRAINT_FAILED
(valided in main.type_json test).
Consequential changes in debug build to json acl
testing validation and unit test having a dummy json_engine_t.
json_normalize also uses the same engine that was
passed as its argument to test json_valid. Because
the character set may have changed, if there's an error
adjust the je->s.c_str pointer. Its only the offset that
is currently used in the error messages anyway so there
may be offset errors depending on the original character
set.
The database name in GRANT is stored in mysql.db.Db, which holds 64
characters. Escaping '_' or '%' with a backslash to match them literally
can make the stored name longer than 64 characters, and it was silently
truncated into a non-functional grant. Reject it with ER_WRONG_DB_NAME
instead, the same error CREATE DATABASE gives for an over-long name.
Add support for CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA in COM_CHANGE_USER
packet parsing, allowing passwords >= 251 bytes.
Changes:
- Server: parse_com_change_user_packet() now handles LENENC-encoded
password length when CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA is set
- Server: fix db pointer calculation for old protocol without
CLIENT_SECURE_CONNECTION (need +1 to skip null terminator)
- Add regression test for COM_CHANGE_USER with long passwords
Note: This fix requires a corresponding client-side fix in libmariadb's
send_change_user_packet() to send LENENC-encoded passwords.
Remove the unscoped #pragma GCC diagnostic ignored
"-Wdeprecated-declarations" from include/violite.h and replace all
sprintf/vsprintf calls with snprintf/vsnprintf across the codebase.
Where possible, pass actual buffer sizes as function parameters
instead of using hardcoded constants or recomputing strlen().
Notable API changes:
- get_date(): added to_len parameter for buffer size
- nice_time(), end_timer() in client/mysql.cc: added buff_size
- generate_new_name(): added name_size parameter
- calc_md5(): added buffer_size parameter (fixes off-by-one)
- MD5_HASH_TO_STRING macro: added _size parameter
- Added MYSQL_UDF_MAX_RESULT_LENGTH constant in mysql_com.h
Also fixed a pre-existing bug in sql/rpl_mi.cc where the ellipsis
"..." was written to the wrong buffer (dbuff instead of buff).
Vendored code is excluded: extra/readline, extra/wolfssl,
wsrep-lib, libmariadb, storage/mroonga/vendor, zlib.
* fail acl_load() if it was killed, this will cause all privileges to
be reset to their original pre-load values.
* only increment grant_version if privileges were, in fact, updated
When GRANT EXECUTE ON PROCEDURE fails on the master, it will
erroneously be replicated and executed successfully on the slave.
This both breaks replication and is a security violation.
The underlying issue is that a failed GRANT EXECUTE ON PROCEDURE will
still be replicated when sql_mode does not have NO_AUTO_CREATE_USER.
This is because the function mysql_routine_grant() does not check if an
error occured while performing the GRANT before binlogging, it simply
always binlogs.
This patch fixes this problem by checking if an error happened
previously before binlogging, and if so, then skip binlogging.
Note there is still a broader issue in this area leading to replication
divergence. Reported in MDEV-29848, a partially-completed GRANT
statment (where some earlier GRANTS succeed and a later fails) will not
binlog. Note this affects all grant types, whereas the issue addressed
in this patch is limited to GRANT EXECUTE ON PROCEDURE. This patch
makes GRANT EXECUTE ON PROCEDURE binlogging behavior consistent with
the other grant types. A separate follow-up patch will address the
broader MDEV-29848 issue.
Also note that a test case in rpl_do_grant.test took advantage of
MDEV-38506 so a partially-failing REVOKE EXECUTE ON PROCEDURE would
still replicate. This test case is disabled with a TODO note to
re-enable it once MDEV-29848 is fixed
Reviewed-by: Sergei Golubchik <serg@mariadb.org>
Signed-off-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
When a client connects with CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA
capability and a password >= 251 bytes, the server incorrectly
calculates the database name pointer.
For passwords >= 251 bytes, LENENC uses a 3-byte prefix (0xFC + 2 bytes),
but the old code assumed a 1-byte prefix. Fix by using the passwd pointer
which has already been advanced past the length prefix by
safe_net_field_length_ll().
Also fix db pointer calculation for old protocol (!CLIENT_SECURE_CONNECTION)
where the password is null-terminated and needs +1 to skip the terminator.
JSON_TABLE is marked as a special "*any_db*" table. Because this special
marking is processed all the way though to get_column_grant where
its processed like its in database called "*any_db*". As this
doesn't exist, only those users with global privileges can create
views on a JSON_TABLE.
Under a Prepared Statement protocol a Create_tmp_table is
used for the JSON_TABLE, but it gets assigned an "" database
name.
We correct this to give it "*any_db*" like the SQL parser
indicating that no database is needed.
To commonly correct the fill_effective_table_privileges
by looking explictly for the "*any_db*", those tables that
have this as the database name get SELECT privileges.
While correcting the database for the JSON_TABLE, lets give
it a "json_table" name rather than "(temporary)" for
greater clarity in warning messages.
check that privilege tables have a PK
and it has the correct number ok key parts
also fixes:
* MDEV-24206 SIGSEGV in replace_db_table on GRANT
* MDEV-24814 SIGSEGV in replace_table_table on GRANT
* MDEV-27842 SIGSEGV in replace_routine_table on GRANT
* MDEV-28128 SIGSEGV in replace_column_table on GRANT
* MDEV-27893 SIGSEGV in replace_proxies_priv_table on GRANT PROXY
json_unescape can return negative numbers, and with
so we should free the buffer allocated.
Also handle the NULL value in unsafe_str by not de-referencing
NULL.
insufficient grants
Defer privilege checking until fix_fields. This way ALTER will behave
consistently with CREATE, and require the same privileges to sequence
column (SELECT/INSERT)
check sequence privileges in Item_func_nextval::fix_fields(),
just like column privileges are checked in Item_field::fix_fields()
remove sequence specific hacks that kinda made sequence privilege
checks works, but not in all cases. And they were too lax,
didn't requre SELECT privilege for NEXTVAL. Also INSERT privilege looks
wrong here, UPDATE would've been more appropriate, but won't
change that for compatibility reasons.
also fixes
MDEV-36413 User without any privileges to a sequence can read from it and modify it via column default
Attempt to create a procedure with the DEFINER clause resulted in
abnormal server termination in case the server run with the option
--skip-grant-tables=1.
The reason of abnormal termination is that on handling of the DEFINER
clause, not initialized data members of acl_cache is accessed, that led
to server crash.
Behaviour of the server for considered use case must be the same
as for embedded server. Than means, if a security subsytem wasn't
initialized (server is started with the option --skip-grant-tables=1)
return success from get_current_user() without further access to the
acl_cache that obviously not initialized.
Additionlly, AUTHID::is_role was modified to handle the case when
a host part of the user name isn't provided. Treat this case as if
the empty host name is provided.
- Needless engaged_ removed;
- SCOPE_VALUE, SCOPE_SET, SCOPE_CLEAR macros for neater declaration;
- IF_CLASS / IF_NOT_CLASS SFINAE checkers to pass arg by value or
reference;
- inline keyword;
- couple of refactorings of temporary free_list.
Example:
{
auto _= make_scope_value(var, tmp_value);
}
make_scope_value(): a function which returns RAII object which temporary
changes a value of a variable
detail::Scope_value: actual implementation of such RAII class.
It shouldn't be used directly! That's why it's inside a namespace detail.
get_all_tables() skipped tables if the user has no privileges on
the schema itself and no granted privilege on any tables in the schema.
that is, it was skipping performance_schema tables (privileges
on them aren't explicitly granted, but internally hard-coded)
To fix:
* extend ACL_internal_table_access::check() method with
`bool any_combination_will_do`
* fix all perfschema privilege checks to take it into account.
* don't reuse table_acl_check object for all tables, initialize it
for every table otherwise GRANT_INTERNAL_INFO will leak
* remove incorrect privilege check from get_all_tables()
during FLUSH PRIVILEGES, allow_all_hosts temporarily goes out of sync
with acl_check_hosts and acl_wild_hosts.
As it's tested in acl_check_host() without a mutex, let's re-test it
under a mutex to make sure the value is correct.
Note that it's just an optimization and it's ok to see outdated
allow_all_hosts value here.
