feat(inputs.sqlserver): Check SQL Server "encryptionEnforce" with xp_instance_regread

[MartinHBA]

• Updated associated README.md.
• Wrote appropriate unit tests.
• Pull request title or commits are in conventional commit format

resolves #12933

Added new query that collects required info from SQL Server registry

[powersj]

@Trovalo could I request your sql server knowledge once again :) and take a quick look at this one? Thanks!

[Trovalo]

I'm not that knowledgeable in this particular area of SQL Server but I'll have a look at it.

[srebhan]

Looks good to me. Thanks for your contribution @MartinHBA! Tagging in @Trovalo to comment on the SQL statement. ;-)

[Trovalo]

Once the SQL statement it's fixed the whole thing should work (also permission-wise), BUT

I'm not sure about having a whole query about it, it's just too specific, therefore I see mainly two options:

• having a more generic "Registry" query to fetch registry values (I'm not sure how many useful registry values there are tho)
• add this new field in an existing query, like the ServerProperties one

I'll think about it some more...

[Trovalo]

This query won't run, single quotes have to be escaped inside the string itself (' → '' ), and the output variable has to be declared inside the statement itself in order to have the proper scope.
also it's missing the instance name, and you won't be able to distinguish between multiple servers without it

SET DEADLOCK_PRIORITY -10;
DECLARE @SqlStatement nvarchar(max)

SET @SqlStatement = N'
DECLARE @ForceEncryption INT

EXEC [master].[dbo].[xp_instance_regread]
       @rootkey = ''HKEY_LOCAL_MACHINE'',
       @key = ''SOFTWARE\Microsoft\Microsoft SQL Server\MSSQLServer\SuperSocketNetLib'',
       @value_name = ''ForceEncryption'',
       @value = @ForceEncryption OUTPUT;

SELECT @@SERVERNAME AS sql_instance, @ForceEncryption AS ForceEncryption'

EXEC sp_executesql @SqlStatement

[MartinHBA]

• having a more generic "Registry" query to fetch registry values (I'm not sure how many useful registry values there are tho)

exactly my thought when I was creating it, it could be more generic registry reader function and for example it can also read SQL port numbers from registry in future and much more, let me know your decision and what naming / changes you want to do, we can close this PR and I will create new incorporating your suggestions

[Trovalo]

@MartinHBA I've thought about having a new query about "Registry" values vs adding stuff to the existing one (ServerProperties).

As long as need few values I think it makes more sense to add new columns to the "ServerProperties" one, I've tested it and it doesn't look ugly.
Also I don't think there are many values worth taking from the registry... is there something worth taking other than

• ForceEncryption
• Port
• PortType (Static/Dynamic)

Let me know if you think there are more things to fetch from the registry (SQL Server related ofc) as I've kind of ended my useful list with just 3 attributes...
Let me know what you think

Here is the whole query, just to give you an idea/sample

SET DEADLOCK_PRIORITY -10;
IF SERVERPROPERTY('EngineEdition') NOT IN (2,3,4) BEGIN /*NOT IN Standard,Enterpris,Express*/
	DECLARE @ErrorMessage AS nvarchar(500) = 'Telegraf - Connection string Server:'+ @@ServerName + ',Database:' + DB_NAME() +' is not a SQL Server Standard,Enterprise or Express. Check the database_type parameter in the telegraf configuration.';
	RAISERROR (@ErrorMessage,11,1)
	RETURN
END

DECLARE
	 @SqlStatement AS nvarchar(max) = ''
	,@MajorMinorVersion AS int = CAST(PARSENAME(CAST(SERVERPROPERTY('ProductVersion') AS nvarchar),4) AS int)*100 + CAST(PARSENAME(CAST(SERVERPROPERTY('ProductVersion') AS nvarchar),3) AS int)
	,@Columns AS nvarchar(MAX) = ''

IF CAST(SERVERPROPERTY('ProductVersion') AS varchar(50)) >= '10.50.2500.0'
	SET @Columns = N'
	,CASE [virtual_machine_type_desc]
		WHEN ''NONE'' THEN ''PHYSICAL Machine''
		ELSE [virtual_machine_type_desc]
	END AS [hardware_type]'

SET @SqlStatement = '

DECLARE @ForceEncryption INT
DECLARE @DynamicportNo NVARCHAR(50);
DECLARE @StaticportNo NVARCHAR(50);

EXEC [master].[dbo].[xp_instance_regread]
       @rootkey = ''HKEY_LOCAL_MACHINE'',
       @key = ''SOFTWARE\Microsoft\Microsoft SQL Server\MSSQLServer\SuperSocketNetLib'',
       @value_name = ''ForceEncryption'',
       @value = @ForceEncryption OUTPUT;

EXEC xp_instance_regread @rootkey = ''HKEY_LOCAL_MACHINE''
                        ,@key =
                         ''Software\Microsoft\Microsoft SQL Server\MSSQLServer\SuperSocketNetLib\Tcp\IpAll''
                        ,@value_name = ''TcpDynamicPorts''
                        ,@value = @DynamicportNo OUTPUT

EXEC xp_instance_regread @rootkey = ''HKEY_LOCAL_MACHINE''
                        ,@key =
                         ''Software\Microsoft\Microsoft SQL Server\MSSQLServer\SuperSocketNetLib\Tcp\IpAll''
                        ,@value_name = ''TcpPort''
                        ,@value = @StaticportNo OUTPUT

SELECT
	 ''sqlserver_server_properties'' AS [measurement]
	,REPLACE(@@SERVERNAME,''\'','':'') AS [sql_instance]
	,@@SERVICENAME AS [service_name]
	,si.[cpu_count]
	,(SELECT [total_physical_memory_kb] FROM sys.[dm_os_sys_memory]) AS [server_memory]
	,(SELECT [available_physical_memory_kb] FROM sys.[dm_os_sys_memory]) AS [available_server_memory]
	,SERVERPROPERTY(''Edition'') AS [sku]
	,CAST(SERVERPROPERTY(''EngineEdition'') AS int) AS [engine_edition]
	,DATEDIFF(MINUTE,si.[sqlserver_start_time],GETDATE()) AS [uptime]
	,SERVERPROPERTY(''ProductVersion'') AS [sql_version]
	,SERVERPROPERTY(''IsClustered'') AS [instance_type]
	,LEFT(@@VERSION,CHARINDEX('' - '',@@VERSION)) AS [sql_version_desc]
	,@ForceEncryption AS ForceEncryption
	,COALESCE(@DynamicportNo,@StaticportNo) AS Port
	,IIF(@DynamicportNo IS NULL, ''Static'', ''Dynamic'') AS PortType
	,dbs.[db_online]
	,dbs.[db_restoring]
	,dbs.[db_recovering]
	,dbs.[db_recoveryPending]
	,dbs.[db_suspect]
	,dbs.[db_offline]'
	+ @Columns + N'
	FROM sys.[dm_os_sys_info] AS si
	CROSS APPLY (
		SELECT
			 SUM(CASE WHEN state = 0 THEN 1 ELSE 0 END) AS [db_online]
			,SUM(CASE WHEN state = 1 THEN 1 ELSE 0 END) AS [db_restoring]
			,SUM(CASE WHEN state = 2 THEN 1 ELSE 0 END) AS [db_recovering]
			,SUM(CASE WHEN state = 3 THEN 1 ELSE 0 END) AS [db_recoveryPending]
			,SUM(CASE WHEN state = 4 THEN 1 ELSE 0 END) AS [db_suspect]
			,SUM(CASE WHEN state IN(6, 10) THEN 1 ELSE 0 END) AS [db_offline]
		FROM sys.databases
	) AS dbs
'

EXEC sp_executesql @SqlStatement

[MartinHBA]

hi @Trovalo , updated now, please check and let me know, also let me thank you for your valuable inputs and your time!

[Trovalo]

I've only formatted the SQL code coherently (commas, brackets, etc) as my sample had been put together quickly (and it was used as-is)

After that we are good to go, thanks @MartinHBA

[telegraf-tiger]

Download PR build artifacts for linux_amd64.tar.gz, darwin_amd64.tar.gz, and windows_amd64.zip.
Downloads for additional architectures and packages are available below.

☺️ This pull request doesn't significantly change the Telegraf binary size (less than 1%)

📦 Click here to get additional PR build artifacts

Artifact URLs

DEB	RPM	TAR GZ	ZIP
amd64.deb	aarch64.rpm	darwin_amd64.tar.gz	windows_amd64.zip
arm64.deb	armel.rpm	darwin_arm64.tar.gz	windows_arm64.zip
armel.deb	armv6hl.rpm	freebsd_amd64.tar.gz	windows_i386.zip
armhf.deb	i386.rpm	freebsd_armv7.tar.gz
i386.deb	ppc64le.rpm	freebsd_i386.tar.gz
mips.deb	riscv64.rpm	linux_amd64.tar.gz
mipsel.deb	s390x.rpm	linux_arm64.tar.gz
ppc64el.deb	x86_64.rpm	linux_armel.tar.gz
riscv64.deb		linux_armhf.tar.gz
s390x.deb		linux_i386.tar.gz
		linux_mips.tar.gz
		linux_mipsel.tar.gz
		linux_ppc64le.tar.gz
		linux_riscv64.tar.gz
		linux_s390x.tar.gz

[powersj]

Thank you both!