From: Terry Burton <tez@terryburton.co.uk>
Date: Tue, 3 Aug 2021 21:17:04 +0000 (+0100)
Subject: MySQL sqlippool SP: Run as invoker, not definer; close transaction on error (#4170)
X-Git-Tag: release_3_0_24~72
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=39d7477610bdc73cba26fdc3b8906e62b71af7b7;p=thirdparty%2Ffreeradius-server.git

MySQL sqlippool SP: Run as invoker, not definer; close transaction on error (#4170)

In MariaDB/MySQL, stored procedures default to running in the context of
the definer rather than the invoker.

This is a problem in a streaming replication scenario since the definer
is often the root user who has the "super" power to write to a read-only
database (unless super-read-only is enabled, which is not available for
MariaDB), thus breaking the replication timeline.

Additionally, exiting an SP does not finalise any running transaction.
If an exception is raised within the SP (e.g. due to the database being
read-only) we must handle this and finalise the transaction, otherwise
subsequent calls to "SET TRANSACTION ISOLATION LEVEL READ COMMITTED"
will fail ad nauseam until the connection is finally closed.
---

diff --git a/raddb/mods-config/sql/ippool-dhcp/mysql/procedure-no-skip-locked.sql b/raddb/mods-config/sql/ippool-dhcp/mysql/procedure-no-skip-locked.sql
index 8d919c711ac..bee37de4abd 100644
--- a/raddb/mods-config/sql/ippool-dhcp/mysql/procedure-no-skip-locked.sql
+++ b/raddb/mods-config/sql/ippool-dhcp/mysql/procedure-no-skip-locked.sql
@@ -50,6 +50,7 @@ CREATE PROCEDURE fr_allocate_previous_or_new_framedipaddress (
 	IN v_lease_duration INT,
 	IN v_requested_address VARCHAR(15)
 )
+SQL SECURITY INVOKER
 proc:BEGIN
 	DECLARE r_address VARCHAR(15);
 
diff --git a/raddb/mods-config/sql/ippool-dhcp/mysql/procedure.sql b/raddb/mods-config/sql/ippool-dhcp/mysql/procedure.sql
index d3f9dd5d2c4..b5dfae08780 100644
--- a/raddb/mods-config/sql/ippool-dhcp/mysql/procedure.sql
+++ b/raddb/mods-config/sql/ippool-dhcp/mysql/procedure.sql
@@ -38,9 +38,16 @@ CREATE PROCEDURE fr_dhcp_allocate_previous_or_new_framedipaddress (
 	IN v_lease_duration INT,
 	IN v_requested_address VARCHAR(15)
 )
+SQL SECURITY INVOKER
 proc:BEGIN
 	DECLARE r_address VARCHAR(15);
 
+	DECLARE EXIT HANDLER FOR SQLEXCEPTION
+	BEGIN
+		ROLLBACK;
+		RESIGNAL;
+	END;
+
 	SET TRANSACTION ISOLATION LEVEL READ COMMITTED;
 
 	START TRANSACTION;
diff --git a/raddb/mods-config/sql/ippool/mysql/procedure-no-skip-locked.sql b/raddb/mods-config/sql/ippool/mysql/procedure-no-skip-locked.sql
index d2a53cae6fd..1c884465430 100644
--- a/raddb/mods-config/sql/ippool/mysql/procedure-no-skip-locked.sql
+++ b/raddb/mods-config/sql/ippool/mysql/procedure-no-skip-locked.sql
@@ -54,6 +54,7 @@ CREATE PROCEDURE fr_allocate_previous_or_new_framedipaddress (
         IN v_pool_key VARCHAR(64),
         IN v_lease_duration INT
 )
+SQL SECURITY INVOKER
 proc:BEGIN
         DECLARE r_address VARCHAR(15);
 
diff --git a/raddb/mods-config/sql/ippool/mysql/procedure.sql b/raddb/mods-config/sql/ippool/mysql/procedure.sql
index 2e88e93d349..121451fb6a1 100644
--- a/raddb/mods-config/sql/ippool/mysql/procedure.sql
+++ b/raddb/mods-config/sql/ippool/mysql/procedure.sql
@@ -42,9 +42,16 @@ CREATE PROCEDURE fr_allocate_previous_or_new_framedipaddress (
         IN v_pool_key VARCHAR(64),
         IN v_lease_duration INT
 )
+SQL SECURITY INVOKER
 proc:BEGIN
         DECLARE r_address VARCHAR(15);
 
+        DECLARE EXIT HANDLER FOR SQLEXCEPTION
+        BEGIN
+            ROLLBACK;
+            RESIGNAL;
+        END;
+
         SET TRANSACTION ISOLATION LEVEL READ COMMITTED;
 
         START TRANSACTION;