From 8c7d8b716c976d28b3373510543fa88f6d12a643 Mon Sep 17 00:00:00 2001 From: Thirunarayanan Balathandayuthapani Date: Tue, 15 Jun 2021 13:12:15 +0530 Subject: [PATCH] MDEV-14180 Automatically disable key rotation checks for file_key_managment plugin Problem: ======= - InnoDB iterates the fil_system space list to encrypt the tablespace in case of key rotation. But it is not necessary for any encryption plugin which doesn't do key version rotation. Solution: ========= - Introduce a new variable called srv_encrypt_rotate to indicate whether encryption plugin does key rotation fil_space_crypt_t::key_get_latest_version(): Enable the srv_encrypt_rotate only once if current key version is higher than innodb_encyrption_rotate_key_age fil_crypt_must_default_encrypt(): Default encryption tables should be added to default_encryp_tables list if innodb_encyrption_rotate_key_age is zero and encryption plugin doesn't do key version rotation fil_space_create(): Add the newly created space to default_encrypt_tables list if fil_crypt_must_default_encrypt() returns true Removed the nondeterministic select from innodb-key-rotation-disable test. By default, InnoDB adds the tablespace to the rotation list and background crypt thread does encryption of tablespace. So these select doesn't give reliable results. --- .../r/innodb-key-rotation-disable.result | 4 -- .../encryption/r/key_version_rotation.result | 19 +++++++++ .../t/innodb-key-rotation-disable.test | 3 -- .../encryption/t/key_version_rotation.opt | 2 + .../encryption/t/key_version_rotation.test | 41 +++++++++++++++++++ storage/innobase/fil/fil0crypt.cc | 20 ++++++++- storage/innobase/fil/fil0fil.cc | 18 +++++--- storage/innobase/include/fil0crypt.h | 6 +++ 8 files changed, 98 insertions(+), 15 deletions(-) create mode 100644 mysql-test/suite/encryption/r/key_version_rotation.result create mode 100644 mysql-test/suite/encryption/t/key_version_rotation.opt create mode 100644 mysql-test/suite/encryption/t/key_version_rotation.test diff --git a/mysql-test/suite/encryption/r/innodb-key-rotation-disable.result b/mysql-test/suite/encryption/r/innodb-key-rotation-disable.result index a662f5e6343..7d0267d5057 100644 --- a/mysql-test/suite/encryption/r/innodb-key-rotation-disable.result +++ b/mysql-test/suite/encryption/r/innodb-key-rotation-disable.result @@ -1,7 +1,3 @@ -SELECT NAME FROM INFORMATION_SCHEMA.INNODB_TABLESPACES_ENCRYPTION WHERE MIN_KEY_VERSION <> 0; -NAME -SELECT NAME FROM INFORMATION_SCHEMA.INNODB_TABLESPACES_ENCRYPTION WHERE MIN_KEY_VERSION = 0; -NAME create database enctests; use enctests; create table t1(a int not null primary key, b char(200)) engine=innodb; diff --git a/mysql-test/suite/encryption/r/key_version_rotation.result b/mysql-test/suite/encryption/r/key_version_rotation.result new file mode 100644 index 00000000000..1a295abfe8d --- /dev/null +++ b/mysql-test/suite/encryption/r/key_version_rotation.result @@ -0,0 +1,19 @@ +create table t1(f1 int not null)engine=innodb; +create table t2(f1 int not null)engine=innodb; +insert into t1 select * from seq_1_to_100; +insert into t2 select * from seq_1_to_100; +# Enable encryption +set global innodb_encrypt_tables=ON; +# Create a new table and it is added to rotation list +create table t3(f1 int not null)engine=innodb; +insert into t3 select * from seq_1_to_100; +# Increase the version and it should set rotation +# variable for the encryption plugin +set global debug_key_management_version=10; +select @@debug_key_management_version; +@@debug_key_management_version +10 +# Decrease the key version and Disable the encryption +set global debug_key_management_version=1; +set global innodb_encrypt_tables=off; +DROP TABLE t1, t2, t3; diff --git a/mysql-test/suite/encryption/t/innodb-key-rotation-disable.test b/mysql-test/suite/encryption/t/innodb-key-rotation-disable.test index 2e5d877f7c0..4541bfa2a0c 100644 --- a/mysql-test/suite/encryption/t/innodb-key-rotation-disable.test +++ b/mysql-test/suite/encryption/t/innodb-key-rotation-disable.test @@ -3,9 +3,6 @@ # not embedded because of restarts -- source include/not_embedded.inc -SELECT NAME FROM INFORMATION_SCHEMA.INNODB_TABLESPACES_ENCRYPTION WHERE MIN_KEY_VERSION <> 0; -SELECT NAME FROM INFORMATION_SCHEMA.INNODB_TABLESPACES_ENCRYPTION WHERE MIN_KEY_VERSION = 0; - --disable_query_log --disable_warnings let $encryption = `SELECT @@innodb_encrypt_tables`; diff --git a/mysql-test/suite/encryption/t/key_version_rotation.opt b/mysql-test/suite/encryption/t/key_version_rotation.opt new file mode 100644 index 00000000000..d7933f0f943 --- /dev/null +++ b/mysql-test/suite/encryption/t/key_version_rotation.opt @@ -0,0 +1,2 @@ +--innodb-tablespaces-encryption +--plugin-load-add=$DEBUG_KEY_MANAGEMENT_SO diff --git a/mysql-test/suite/encryption/t/key_version_rotation.test b/mysql-test/suite/encryption/t/key_version_rotation.test new file mode 100644 index 00000000000..d36d47251a1 --- /dev/null +++ b/mysql-test/suite/encryption/t/key_version_rotation.test @@ -0,0 +1,41 @@ +--source include/have_innodb.inc +--source include/have_debug.inc +--source include/have_sequence.inc + +create table t1(f1 int not null)engine=innodb; +create table t2(f1 int not null)engine=innodb; +insert into t1 select * from seq_1_to_100; +insert into t2 select * from seq_1_to_100; + +let $restart_parameters=--innodb_encrypt_tables=0 --innodb_encryption_threads=1 --innodb_encryption_rotate_key_age=9; +--source include/restart_mysqld.inc + +--echo # Enable encryption + +set global innodb_encrypt_tables=ON; +--let $tables_count= `select count(*) from information_schema.tables where engine = 'InnoDB'` +--let $wait_timeout= 600 +--let $wait_condition=SELECT COUNT(*) >= $tables_count FROM INFORMATION_SCHEMA.INNODB_TABLESPACES_ENCRYPTION WHERE MIN_KEY_VERSION <> 0; +--source include/wait_condition.inc +--echo # Create a new table and it is added to rotation list +create table t3(f1 int not null)engine=innodb; +insert into t3 select * from seq_1_to_100; + +--echo # Increase the version and it should set rotation +--echo # variable for the encryption plugin + +set global debug_key_management_version=10; +select @@debug_key_management_version; +--let $tables_count= `select count(*) from information_schema.tables where engine = 'InnoDB'` +--let $wait_timeout= 600 +--let $wait_condition=SELECT COUNT(*) >= $tables_count FROM INFORMATION_SCHEMA.INNODB_TABLESPACES_ENCRYPTION WHERE MIN_KEY_VERSION <> 0; +--source include/wait_condition.inc + +--echo # Decrease the key version and Disable the encryption +set global debug_key_management_version=1; +set global innodb_encrypt_tables=off; + +--let $wait_timeout= 600 +--let $wait_condition=SELECT COUNT(*) >= $tables_count FROM INFORMATION_SCHEMA.INNODB_TABLESPACES_ENCRYPTION WHERE MIN_KEY_VERSION = 0; +--source include/wait_condition.inc +DROP TABLE t1, t2, t3; diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc index b167a589695..8195716f722 100644 --- a/storage/innobase/fil/fil0crypt.cc +++ b/storage/innobase/fil/fil0crypt.cc @@ -55,6 +55,9 @@ UNIV_INTERN uint srv_n_fil_crypt_threads_started = 0; /** At this age or older a space/page will be rotated */ UNIV_INTERN uint srv_fil_crypt_rotate_key_age; +/** Whether the encryption plugin does key rotation */ +static bool srv_encrypt_rotate; + /** Event to signal FROM the key rotation threads. */ static os_event_t fil_crypt_event; @@ -136,6 +139,14 @@ fil_space_crypt_t::key_get_latest_version(void) if (is_key_found()) { key_version = encryption_key_get_latest_version(key_id); + /* InnoDB does dirty read of srv_fil_crypt_rotate_key_age. + It doesn't matter because srv_encrypt_rotate + can be set to true only once */ + if (!srv_encrypt_rotate + && key_version > srv_fil_crypt_rotate_key_age) { + srv_encrypt_rotate = true; + } + srv_stats.n_key_requests.inc(); key_found = key_version; } @@ -1380,6 +1391,11 @@ fil_crypt_return_iops( fil_crypt_update_total_stat(state); } +bool fil_crypt_must_default_encrypt() +{ + return !srv_fil_crypt_rotate_key_age || !srv_encrypt_rotate; +} + /** Return the next tablespace from default_encrypt_tables. @param space previous tablespace (NULL to start from the start) @param recheck whether the removal condition needs to be rechecked after @@ -1455,7 +1471,7 @@ static fil_space_t *fil_space_next(fil_space_t *space, bool recheck, mutex_enter(&fil_system->mutex); ut_ad(!space || space->n_pending_ops); - if (!srv_fil_crypt_rotate_key_age) + if (fil_crypt_must_default_encrypt()) space= fil_system->default_encrypt_next(space, recheck, encrypt); else if (!space) { @@ -2448,7 +2464,7 @@ fil_crypt_set_encrypt_tables( srv_encrypt_tables = val; - if (srv_fil_crypt_rotate_key_age == 0) { + if (fil_crypt_must_default_encrypt()) { fil_crypt_default_encrypt_tables_fill(); } diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc index 3dc804ea878..a727215b433 100644 --- a/storage/innobase/fil/fil0fil.cc +++ b/storage/innobase/fil/fil0fil.cc @@ -1464,22 +1464,28 @@ fil_space_create( fil_system->max_assigned_id = id; } + const bool rotate = + (purpose == FIL_TYPE_TABLESPACE + && (mode == FIL_ENCRYPTION_ON + || mode == FIL_ENCRYPTION_OFF || srv_encrypt_tables) + && fil_crypt_must_default_encrypt()); + /* Inform key rotation that there could be something to do */ - if (purpose == FIL_TYPE_TABLESPACE - && !srv_fil_crypt_rotate_key_age && fil_crypt_threads_event && - (mode == FIL_ENCRYPTION_ON || mode == FIL_ENCRYPTION_OFF || - srv_encrypt_tables)) { + if (rotate) { /* Key rotation is not enabled, need to inform background encryption threads. */ fil_system->default_encrypt_tables.push_back(*space); space->is_in_default_encrypt = true; mutex_exit(&fil_system->mutex); + } else { + mutex_exit(&fil_system->mutex); + } + + if (rotate && srv_n_fil_crypt_threads_started) { mutex_enter(&fil_crypt_threads_mutex); os_event_set(fil_crypt_threads_event); mutex_exit(&fil_crypt_threads_mutex); - } else { - mutex_exit(&fil_system->mutex); } return(space); diff --git a/storage/innobase/include/fil0crypt.h b/storage/innobase/include/fil0crypt.h index 3c56315ee9a..af6c930659b 100644 --- a/storage/innobase/include/fil0crypt.h +++ b/storage/innobase/include/fil0crypt.h @@ -499,4 +499,10 @@ bool fil_space_verify_crypt_checksum(const byte* page, const page_size_t& page_size) MY_ATTRIBUTE((warn_unused_result)); +/** Add the tablespace to the rotation list if +innodb_encrypt_rotate_key_age is 0 or encryption plugin does +not do key version rotation +@return whether the tablespace should be added to rotation list */ +bool fil_crypt_must_default_encrypt(); + #endif /* fil0crypt_h */