Fixed #8429: Segfault when already destroyed callback interface is used

Author: AlexPeshkoff

doc/Using_OO_API.html

@@ -3629,13 +3629,24 @@ <h1><font size="4" style="font-size: 14pt">Database encryption
 <p style="margin-bottom: 0cm"><font size="4" style="font-size: 14pt">DO_RETRY -
 	retry attach (ignored when function was called without attStatus).</font></p>
 	</font></p>
+	<li><p style="margin-bottom: 0cm"><font size="4" style="font-size: 14pt">
+	int getHashLength(StatusType* status) - returns length of hash of the keys provided by interface.
+	Hash is needed to let caller compare interfaces for equality. Empty (no keys)
+	interface should return hash length equal to zero.
+	</font></p>
+	<li><p style="margin-bottom: 0cm"><font size="4" style="font-size: 14pt">
+	void getHashData(StatusType* status, void* hash) - places hash of the keys provided by interface
+	into memory location defined by parameter <i>hash</i>. Should copy exact number of bytes
+	returned by getHashLength() call. Hash algorithm is chosen by plugin author
+	(in year 2024 SHA256 appears reasonable choice).
+	</font></p>
 </ol>
 <p style="margin-bottom: 0cm"><br/>
 
 </p>
 <p style="margin-bottom: 0cm"><a name="DbCryptInfo"></a><font size="4" style="font-size: 14pt">DbCryptInfo
 interface is passed to DbCryptPlugin by engine. Plugin may save this
-interface and use when needed to obtain additional informatio about
+interface and use when needed to obtain additional information about
 database.</font></p>
 <ol>
 	<li><p style="margin-bottom: 0cm"><font size="4" style="font-size: 14pt">const

examples/dbcrypt/CryptApplication.cpp

@@ -36,14 +36,28 @@ class CryptKey : public ICryptKeyCallbackImpl<CryptKey, CheckStatusWrapper>
 	{
 		if (length > 0 && buffer)
 		{
-			char k = 0x5a;
 			memcpy(buffer, &k, 1);
 			fprintf(stderr, "\nTransfered key to server\n");
 		}
 		return 1;
 	}
+
+	int getHashLength(Firebird::CheckStatusWrapper* status) override
+	{
+		return 1;
+	}
+
+	void getHashData(Firebird::CheckStatusWrapper* status, void* h) override
+	{
+		memcpy(h, &k, 1);
+	}
+
+private:
+	static const char k;
 };
 
+const char CryptKey::k = 0x5a;
+
 class App
 {
 public:

examples/dbcrypt/CryptKeyHolder.cpp

@@ -70,7 +70,7 @@ class CryptKeyHolder : public IKeyHolderPluginImpl<CryptKeyHolder, CheckStatusWr
 public:
 	explicit CryptKeyHolder(IPluginConfig* cnf) noexcept
 		: callbackInterface(this), named(NULL), tempStatus(master->getStatus()),
-		  config(cnf), key(0), owner(NULL)
+		  config(cnf), key(0), init(false), owner(NULL)
 	{
 		config->addRef();
 	}
@@ -111,10 +111,13 @@ class CryptKeyHolder : public IKeyHolderPluginImpl<CryptKeyHolder, CheckStatusWr
 		return owner;
 	}
 
-	ISC_UCHAR getKey()
+	const ISC_UCHAR& getKey()
 	{
-		if (!key)
+		if (!init)
+		{
 			keyCallback(&tempStatus, NULL);
+			init = true;
+		}
 
 		return key;
 	}
@@ -140,7 +143,7 @@ class CryptKeyHolder : public IKeyHolderPluginImpl<CryptKeyHolder, CheckStatusWr
 
 		unsigned int callback(unsigned int, const void*, unsigned int length, void* buffer) override
 		{
-			ISC_UCHAR k = holder->getKey();
+			const ISC_UCHAR& k = holder->getKey();
 			if (!k)
 			{
 				return 0;
@@ -153,6 +156,36 @@ class CryptKeyHolder : public IKeyHolderPluginImpl<CryptKeyHolder, CheckStatusWr
 			return 1;
 		}
 
+		int getHashLength(Firebird::CheckStatusWrapper* status) override
+		{
+			const ISC_UCHAR& k = holder->getKey();
+			if (!k)
+			{
+				ISC_STATUS err[] = {isc_arg_gds, isc_wish_list};
+				status->setErrors2(2, err);
+
+				return -1;
+			}
+
+			return 1;
+		}
+
+		void getHashData(Firebird::CheckStatusWrapper* status, void* h) override
+		{
+			// here key value is returned by hash function as is
+			// do not do it in production - use some hash function
+			const ISC_UCHAR& k = holder->getKey();
+			if (!k)
+			{
+				ISC_STATUS err[] = {isc_arg_gds, isc_wish_list};
+				status->setErrors2(2, err);
+
+				return;
+			}
+
+			memcpy(h, &k, 1);
+		}
+
 	private:
 		CryptKeyHolder* holder;
 	};
@@ -173,6 +206,18 @@ class CryptKeyHolder : public IKeyHolderPluginImpl<CryptKeyHolder, CheckStatusWr
 			return 1;
 		}
 
+		int getHashLength(Firebird::CheckStatusWrapper* status) override
+		{
+			return 1;
+		}
+
+		void getHashData(Firebird::CheckStatusWrapper* status, void* h) override
+		{
+			// here key value is returned by hash function as is
+			// do not do it in production - use some hash function
+			memcpy(h, &key, 1);
+		}
+
 		~NamedCallback()
 		{
 			delete next;
@@ -189,6 +234,7 @@ class CryptKeyHolder : public IKeyHolderPluginImpl<CryptKeyHolder, CheckStatusWr
 
 	IPluginConfig* config;
 	ISC_UCHAR key;
+	bool init;
 
 	std::atomic_int refCounter;
 	IReferenceCounted* owner;

src/include/firebird/FirebirdInterface.idl

@@ -943,7 +943,7 @@ interface CryptKeyCallback : Versioned
 	uint callback(uint dataLength, const void* data,
 		uint bufferLength, void* buffer);
 
-version:		// 6.0
+version:		// 6.0, 5.0.2, 4.0.6
 	// Result returned by afterAttach()
 	const uint NO_RETRY		= 0;	// Returned by old plugins & stub
 	const uint DO_RETRY		= 1;
@@ -957,6 +957,12 @@ version:		// 6.0
 	// interface not needed any more
 	[stub defaultAction]
 	void dispose();
+
+version:		// 6.0, 5.0.2, 4.0.6
+	// Produce something to be compared with other interface instance
+	[notImplemented(-1)]
+	int getHashLength(Status status);
+	void getHashData(Status status, void* hash);
 }
 
 

src/include/firebird/IdlFbInterfaces.h

@@ -3892,7 +3892,7 @@ namespace Firebird
 		}
 	};
 
-#define FIREBIRD_ICRYPT_KEY_CALLBACK_VERSION 3u
+#define FIREBIRD_ICRYPT_KEY_CALLBACK_VERSION 4u
 
 	class ICryptKeyCallback : public IVersioned
 	{
@@ -3902,6 +3902,8 @@ namespace Firebird
 			unsigned (CLOOP_CARG *callback)(ICryptKeyCallback* self, unsigned dataLength, const void* data, unsigned bufferLength, void* buffer) CLOOP_NOEXCEPT;
 			unsigned (CLOOP_CARG *afterAttach)(ICryptKeyCallback* self, IStatus* status, const char* dbName, const IStatus* attStatus) CLOOP_NOEXCEPT;
 			void (CLOOP_CARG *dispose)(ICryptKeyCallback* self) CLOOP_NOEXCEPT;
+			int (CLOOP_CARG *getHashLength)(ICryptKeyCallback* self, IStatus* status) CLOOP_NOEXCEPT;
+			void (CLOOP_CARG *getHashData)(ICryptKeyCallback* self, IStatus* status, void* hash) CLOOP_NOEXCEPT;
 		};
 
 	protected:
@@ -3948,6 +3950,33 @@ namespace Firebird
 			}
 			static_cast<VTable*>(this->cloopVTable)->dispose(this);
 		}
+
+		template <typename StatusType> int getHashLength(StatusType* status)
+		{
+			if (cloopVTable->version < 4)
+			{
+				StatusType::setVersionError(status, "ICryptKeyCallback", cloopVTable->version, 4);
+				StatusType::checkException(status);
+				return -1;
+			}
+			StatusType::clearException(status);
+			int ret = static_cast<VTable*>(this->cloopVTable)->getHashLength(this, status);
+			StatusType::checkException(status);
+			return ret;
+		}
+
+		template <typename StatusType> void getHashData(StatusType* status, void* hash)
+		{
+			if (cloopVTable->version < 4)
+			{
+				StatusType::setVersionError(status, "ICryptKeyCallback", cloopVTable->version, 4);
+				StatusType::checkException(status);
+				return;
+			}
+			StatusType::clearException(status);
+			static_cast<VTable*>(this->cloopVTable)->getHashData(this, status, hash);
+			StatusType::checkException(status);
+		}
 	};
 
 #define FIREBIRD_IKEY_HOLDER_PLUGIN_VERSION 5u
@@ -14426,6 +14455,8 @@ namespace Firebird
 					this->callback = &Name::cloopcallbackDispatcher;
 					this->afterAttach = &Name::cloopafterAttachDispatcher;
 					this->dispose = &Name::cloopdisposeDispatcher;
+					this->getHashLength = &Name::cloopgetHashLengthDispatcher;
+					this->getHashData = &Name::cloopgetHashDataDispatcher;
 				}
 			} vTable;
 
@@ -14471,6 +14502,35 @@ namespace Firebird
 				StatusType::catchException(0);
 			}
 		}
+
+		static int CLOOP_CARG cloopgetHashLengthDispatcher(ICryptKeyCallback* self, IStatus* status) CLOOP_NOEXCEPT
+		{
+			StatusType status2(status);
+
+			try
+			{
+				return static_cast<Name*>(self)->Name::getHashLength(&status2);
+			}
+			catch (...)
+			{
+				StatusType::catchException(&status2);
+				return static_cast<int>(0);
+			}
+		}
+
+		static void CLOOP_CARG cloopgetHashDataDispatcher(ICryptKeyCallback* self, IStatus* status, void* hash) CLOOP_NOEXCEPT
+		{
+			StatusType status2(status);
+
+			try
+			{
+				static_cast<Name*>(self)->Name::getHashData(&status2, hash);
+			}
+			catch (...)
+			{
+				StatusType::catchException(&status2);
+			}
+		}
 	};
 
 	template <typename Name, typename StatusType, typename Base = IVersionedImpl<Name, StatusType, Inherit<ICryptKeyCallback> > >
@@ -14494,6 +14554,8 @@ namespace Firebird
 		virtual void dispose()
 		{
 		}
+		virtual int getHashLength(StatusType* status) = 0;
+		virtual void getHashData(StatusType* status, void* hash) = 0;
 	};
 
 	template <typename Name, typename StatusType, typename Base>

src/include/gen/Firebird.pas

@@ -492,6 +492,8 @@ 	ISC_TIMESTAMP_TZ_EX = record
 	ICryptKeyCallback_callbackPtr = function(this: ICryptKeyCallback; dataLength: Cardinal; data: Pointer; bufferLength: Cardinal; buffer: Pointer): Cardinal; cdecl;
 	ICryptKeyCallback_afterAttachPtr = function(this: ICryptKeyCallback; status: IStatus; dbName: PAnsiChar; attStatus: IStatus): Cardinal; cdecl;
 	ICryptKeyCallback_disposePtr = procedure(this: ICryptKeyCallback); cdecl;
+	ICryptKeyCallback_getHashLengthPtr = function(this: ICryptKeyCallback; status: IStatus): Integer; cdecl;
+	ICryptKeyCallback_getHashDataPtr = procedure(this: ICryptKeyCallback; status: IStatus; hash: Pointer); cdecl;
 	IKeyHolderPlugin_keyCallbackPtr = function(this: IKeyHolderPlugin; status: IStatus; callback: ICryptKeyCallback): Integer; cdecl;
 	IKeyHolderPlugin_keyHandlePtr = function(this: IKeyHolderPlugin; status: IStatus; keyName: PAnsiChar): ICryptKeyCallback; cdecl;
 	IKeyHolderPlugin_useOnlyOwnKeysPtr = function(this: IKeyHolderPlugin; status: IStatus): Boolean; cdecl;
@@ -2380,16 +2382,20 @@ 	CryptKeyCallbackVTable = class(VersionedVTable)
 		callback: ICryptKeyCallback_callbackPtr;
 		afterAttach: ICryptKeyCallback_afterAttachPtr;
 		dispose: ICryptKeyCallback_disposePtr;
+		getHashLength: ICryptKeyCallback_getHashLengthPtr;
+		getHashData: ICryptKeyCallback_getHashDataPtr;
 	end;
 
 	ICryptKeyCallback = class(IVersioned)
-		const VERSION = 3;
+		const VERSION = 4;
 		const NO_RETRY = Cardinal(0);
 		const DO_RETRY = Cardinal(1);
 
 		function callback(dataLength: Cardinal; data: Pointer; bufferLength: Cardinal; buffer: Pointer): Cardinal;
 		function afterAttach(status: IStatus; dbName: PAnsiChar; attStatus: IStatus): Cardinal;
 		procedure dispose();
+		function getHashLength(status: IStatus): Integer;
+		procedure getHashData(status: IStatus; hash: Pointer);
 	end;
 
 	ICryptKeyCallbackImpl = class(ICryptKeyCallback)
@@ -2398,6 +2404,8 @@ 	ICryptKeyCallbackImpl = class(ICryptKeyCallback)
 		function callback(dataLength: Cardinal; data: Pointer; bufferLength: Cardinal; buffer: Pointer): Cardinal; virtual; abstract;
 		function afterAttach(status: IStatus; dbName: PAnsiChar; attStatus: IStatus): Cardinal; virtual;
 		procedure dispose(); virtual;
+		function getHashLength(status: IStatus): Integer; virtual; abstract;
+		procedure getHashData(status: IStatus; hash: Pointer); virtual; abstract;
 	end;
 
 	KeyHolderPluginVTable = class(PluginBaseVTable)
@@ -8151,6 +8159,29 @@ procedure ICryptKeyCallback.dispose();
 	end;
 end;
 
+function ICryptKeyCallback.getHashLength(status: IStatus): Integer;
+begin
+	if (vTable.version < 4) then begin
+		FbException.setVersionError(status, 'ICryptKeyCallback', vTable.version, 4);
+		Result := -1;
+	end
+	else begin
+		Result := CryptKeyCallbackVTable(vTable).getHashLength(Self, status);
+	end;
+	FbException.checkException(status);
+end;
+
+procedure ICryptKeyCallback.getHashData(status: IStatus; hash: Pointer);
+begin
+	if (vTable.version < 4) then begin
+		FbException.setVersionError(status, 'ICryptKeyCallback', vTable.version, 4);
+	end
+	else begin
+		CryptKeyCallbackVTable(vTable).getHashData(Self, status, hash);
+	end;
+	FbException.checkException(status);
+end;
+
 function IKeyHolderPlugin.keyCallback(status: IStatus; callback: ICryptKeyCallback): Integer;
 begin
 	Result := KeyHolderPluginVTable(vTable).keyCallback(Self, status, callback);
@@ -13507,6 +13538,25 @@ procedure ICryptKeyCallbackImpl.dispose();
 begin
 end;
 
+function ICryptKeyCallbackImpl_getHashLengthDispatcher(this: ICryptKeyCallback; status: IStatus): Integer; cdecl;
+begin
+	Result := 0;
+	try
+		Result := ICryptKeyCallbackImpl(this).getHashLength(status);
+	except
+		on e: Exception do FbException.catchException(status, e);
+	end
+end;
+
+procedure ICryptKeyCallbackImpl_getHashDataDispatcher(this: ICryptKeyCallback; status: IStatus; hash: Pointer); cdecl;
+begin
+	try
+		ICryptKeyCallbackImpl(this).getHashData(status, hash);
+	except
+		on e: Exception do FbException.catchException(status, e);
+	end
+end;
+
 var
 	ICryptKeyCallbackImpl_vTable: CryptKeyCallbackVTable;
 
@@ -17592,10 +17642,12 @@ initialization
 	IWireCryptPluginImpl_vTable.setSpecificData := @IWireCryptPluginImpl_setSpecificDataDispatcher;
 
 	ICryptKeyCallbackImpl_vTable := CryptKeyCallbackVTable.create;
-	ICryptKeyCallbackImpl_vTable.version := 3;
+	ICryptKeyCallbackImpl_vTable.version := 4;
 	ICryptKeyCallbackImpl_vTable.callback := @ICryptKeyCallbackImpl_callbackDispatcher;
 	ICryptKeyCallbackImpl_vTable.afterAttach := @ICryptKeyCallbackImpl_afterAttachDispatcher;
 	ICryptKeyCallbackImpl_vTable.dispose := @ICryptKeyCallbackImpl_disposeDispatcher;
+	ICryptKeyCallbackImpl_vTable.getHashLength := @ICryptKeyCallbackImpl_getHashLengthDispatcher;
+	ICryptKeyCallbackImpl_vTable.getHashData := @ICryptKeyCallbackImpl_getHashDataDispatcher;
 
 	IKeyHolderPluginImpl_vTable := KeyHolderPluginVTable.create;
 	IKeyHolderPluginImpl_vTable.version := 5;