According to the code, the result is according to the standard, rightserg?
Backtraces:

#0  my_message_sql (error=1959, str=0x7ffff7ed8360 "Invalid role specification `test_role`.", MyFlags=0) at /home/anel/mariadb/10.1/sql/mysqld.cc:3507

#1  0x00005555562c8416 in my_error (nr=1959, MyFlags=0) at /home/anel/mariadb/10.1/mysys/my_error.c:125

#2  0x00005555559af12f in check_user_can_set_role (user=0x7fffd0e6a1c0 "test_user", host=0x555556398cad "%", ip=0x555556398cad "%", rolename=0x7fffd0e6a150 "test_role", access=0x0) at /home/anel/mariadb/10.1/sql/sql_acl.cc:2093

#3  0x00005555559b1a5c in acl_set_default_role (thd=0x7fffd0e4c070, host=0x555556398cad "%", user=0x7fffd0e6a1c0 "test_user", rolename=0x7fffd0e6a150 "test_role") at /home/anel/mariadb/10.1/sql/sql_acl.cc:2922

#4  0x0000555555981c0d in set_var_default_role::update (this=0x7fffd0e6a230, thd=0x7fffd0e4c070) at /home/anel/mariadb/10.1/sql/set_var.cc:938

#5  0x0000555555981416 in sql_set_variables (thd=0x7fffd0e4c070, var_list=0x7fffd0e50898, free=true) at /home/anel/mariadb/10.1/sql/set_var.cc:696

#6  0x0000555555a442af in mysql_execute_command (thd=0x7fffd0e4c070) at /home/anel/mariadb/10.1/sql/sql_parse.cc:4077

#7  0x0000555555a4dc4b in mysql_parse (thd=0x7fffd0e4c070, rawbuf=0x7fffd0e6a088 "SET DEFAULT ROLE test_role FOR test_user", length=40, parser_state=0x7ffff7edd1a0) at /home/anel/mariadb/10.1/sql/sql_parse.cc:7209

And in code is said that:

  /* According to SQL standard, the same error message must be presented */

  if (role == NULL) {

    my_error(ER_INVALID_ROLE, MYF(0), rolename);

    result= -1;

    goto end;

  }

  /* According to SQL standard, the same error message must be presented */

  if (!is_granted)

  {

    my_error(ER_INVALID_ROLE, MYF(0), rolename);

    result= 1;

    goto end;

  }

It would be pretty disappointing if the SQL standard mandated an opaque error message in a context where that does not improve security.

I could possibly understand the point of having an opaque error message in the case of an unprivileged user executing SET ROLE, since there is a chance that the user could be trying to determine what roles the system has.

I do not understand the point of having an opaque error message in the case of a privileged user executing SET DEFAULT ROLE for a different unprivileged user--especially if that privileged user has SELECT privileges for the mysql.global_priv or mysql.user table.

Is the SET DEFAULT ROLE statement even defined by the SQL standard? I thought it was an extension to the standard.

SET DEFAULT ROLE is not standard. The standard says (I presume, didn't check right now), that

SET ROLE foo;

should fail with "Invalid role specification" if it was not granted to the user. Independently from whether the role exists or not.

So GeoffMontee we can confirm this issue and start working on implementing patch for set default role without granting

User `test_user`@`%` has not been granted role `test_role`

Let's say, SET DEFAULT ROLE xxx [FOR yyy] should say "User yyy has not been granted a role xxx" if the current user (not the user yyy in the FOR clause) can see the role xxx. Otherwise it should be "Invalid role specification".

In other words, it should not be possible to use SET DEFAULT ROLE to discover whether a specific role exist or not.

Hi serg, I started looking into this.
I'm not sure how to check that current user can see the role (only what I think of this is that after role is created it is visible in information_schema.applicable_roles).
In this scenario, after creating the role with root, record exists in IS.applicable_roles and I have thought it would be enough to do my_hash_search(&acl_roles_mappings, acl_user->user.str, acl_user->user.length) in function check_user_can_set_role, but the result is null.

Hi serg here is a patch 4312f382b6f09d373d

Hi serg here is new patch: 9a792dea7b.
Thanks.

Hi serg,
after cvicentiu's review here is a new patch da95ee91486f6fd
The main difference is that I understood you that if a current user have select_priv and not update_priv than SET DEFAULT ROLE r1 for a should fail with new error, while the latest patch fails with ER_DBACCESS_DENIED_ERROR.

Looks good to me, ok to push.

Because of some failing tests in buildbot , I've tested again locally all to be sure and to report here:

The servers were restarted 1826 times

Spent 14574.749 of 4391 seconds executing testcases

Completed: All 6677 tests were successful.

403 tests were skipped, 299 by the test itself.

Will push, thanks.

Pushed to 10.1 with 957cb7b7ba35518