marko What you think should we change this more allowing or call it works as designed? When encryption is off default key_id must be 1.

The commit message for ef40018535b71f0a4387fa6c1cc22e9991dfc0db says that the encryption key is not saved to the table's metadata anywhere if ENCRYPTED=DEFAULT and innodb_encrypt_tables=OFF, so why does the value of innodb_default_encryption_key_id even matter in this context? Even if innodb_default_encryption_key_id=1, then that value is not going to be saved to the table's metadata, either, right? If that's correct, then it seems like innodb_default_encryption_key_id should be ignored in this case, and the CREATE TABLE should succeed. Either way, the table is not going to be encrypted, and the encryption key is not going to be saved to the table's metadata.

Is it currently possible for ha_innobase::check_table_options to know if a table's ENCRYPTION_KEY_ID was derived from an explicitly specified ENCRYPTION_KEY_ID table option or from the innodb_default_encryption_key_id system variable, with ENCRYPTION_KEY_ID defined as a HA_TOPTION_SYSVAR, like this?:

https://github.com/MariaDB/server/blob/ef40018535b71f0a4387fa6c1cc22e9991dfc0db/storage/xtradb/handler/ha_innodb.cc#L715

I think that I raised the same issue during the review of the previous change.

jplindst, can you please try to find a solution?

https://github.com/MariaDB/server/commit/64de169b7132cced1c3096d43839f55fdda01c2a

GeoffMontee, this is not a bug. MariaDB remembers table options at create time. When you do

SET SESSION innodb_default_encryption_key_id=2;

CREATE TABLE unencrypted_tab (id int primary key);

you are creating a table with ENCRYPTED_KEY_ID=2. Such a key does not exist, and InnoDB throws an error. This is the expected behavior.

An alternative behavior would be not to check for encryption key id validity until the encryption is enabled. But then you could create hundreds or thousands of tables with invalid encryption key ids, and month later somebody else would enable the encryption globally and suddenly there are tons of errors from old tables. And there is no mass-ALTER, so one would have to ALTER them one by one to fix encryption key ids. This is very inconvenient, and not really SQL way (views, stored procedrures, etc, are checked at CREATE time).

The bug was in 10.1.37, that it did not use table's ENCRYPTION_KEY_ID if encryption was enabled later. So 10.1.37 didn't care if the value is valid, because it would be ignored later anyway. First, user specified value was ignored, and second, SHOW CREATE TABLE would show an incorrect encryption key id (not the one the table was actually encrypted with).

What InnoDB could do here, is not to allow to assign an invalid value to innodb_default_encryption_key_id in the first place. This is quite possible to implement.

Hi serg,

Such a key does not exist, and InnoDB throws an error. This is the expected behavior.

I think you are mistaken. This bug report is not about non-existent keys.

you are creating a table with ENCRYPTED_KEY_ID=2. Such a key does not exist, and InnoDB throws an error. This is the expected behavior.

No. That is not what I'm doing. The key exists. The warning that I'm getting is this one:

InnoDB: innodb_encrypt_tables=OFF only allows ENCRYPTION_KEY_ID=1  

Did you look at the commit that introduced this warning?:

https://github.com/MariaDB/server/commit/ef40018535b71f0a4387fa6c1cc22e9991dfc0db#diff-48fb879ad9d11afdfac59268a011ba97

It says that innodb_default_encryption_key_id must be set to 1 if innodb_encrypt_tables=OFF is set. Therefore, if innodb_encrypt_tables=OFF is set, then setting innodb_default_encryption_key_id=2 is invalid, even if a key with ID 2 exists. This restriction doesn't really make sense to me.

The commit message says that this restriction was added because the encryption key ID is not saved to the table's metadata anywhere if ENCRYPTED=DEFAULT is set for the table and innodb_encrypt_tables=OFF is set. But if the encryption key is not saved to the table's metadata, and if the table is not going to be encrypted immediately, then why does it care what innodb_default_encryption_key_id is set to? The table is not going to be encrypted, and it is not going to save the key ID to its metadata, regardless of whether the key ID is 1, is it?

MariaDB remembers table options at create time.

Does it remember the encryption key ID if the table is not immediately encrypted? This seems to contradict the commit message of the commit referenced above.

Right, I misunderstood, sorry. Yes, the server always remembers all table options at create time. InnoDB doesn't remember encryption key id unless the table is encrypted though.

It is a bug, yes. May be difficult to fix, though

http://lists.askmonty.org/pipermail/commits/2019-February/013456.html

I hope to discuss the design with serg today or tomorrow.

I just discussed this with monty. His idea is that "don’t care" attributes should be ignored, or at most a warning could be issued for them. This allows ALTER TABLE from one storage engine to another while preserving attributes that only matter for some storage engines.

Based on that basic principle, we should ignore the encryption_key_id attribute (whether or not it was explicitly specified by the user, or inherited from the session variable innodb_default_encryption_key_id). We could issue a warning if innodb_default_encryption_key_id is assigned to an invalid value, or an invalid encryption_key_id is being implicitly or explicitly passed to CREATE TABLE or ALTER TABLE.

The problem with encryption_key_id is that it is not being persisted anywhere in InnoDB except if the table attribute encryption is specified and is something else than encryption=default. MDEV-17320 made it a hard error if encryption_key_id is specified to be anything else than 1 in that case.

Ideally, we would always persist encryption_key_id in InnoDB. But, then we would have to be prepared for the case that when encryption is being enabled for a table whose encryption_key_id attribute refers to a non-existing key.

Starting with MariaDB Server 10.2, thanks to MDEV-5800, we could open the table definition from InnoDB side when the encryption is being enabled.

In MariaDB Server 10.1, our best option remains to not store anything inside InnoDB. But, instead of returning the error that MDEV-17320 introduced, we should merely issue a warning that the specified encryption_key_id is going to be ignored if encryption=default. So, basically we do not fix the root cause of what was reported in MDEV-17320, and instead of preventing that scenario by reporting an error, we will report a warning to make the user aware that the key will be ignored.

I documented the issues pointed out by marko here:

https://mariadb.com/kb/en/library/innodb-encryption-troubleshooting/#setting-encryption-key-id-for-an-unencrypted-table

And created MDEV-19086 to track the fix.