Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Azure Active Directory (aka Microsoft Entra) authentication for Azure SQL #662

Closed
dimitri-furman opened this issue Feb 3, 2024 · 10 comments · Fixed by #665
Closed
Assignees
Labels
database Database specific enhancement New feature or request

Comments

@dimitri-furman
Copy link

Is your feature request related to a problem? Please describe.
Azure SQL supports two types of authentication, SQL and Azure Active Directory (aka AAD, aka Microsoft Entra). However, it is possible to disable SQL authentication on an Azure SQL logical server. For compliance and security reasons, this is a common configuration these days. That makes it impossible to run HammerDB against databases on such a server.

Describe the solution you'd like
The MSSQL ODBC driver supports AAD authentication. If, in addition to SQL and Windows, HammerDB adds support for AAD authentication, the problem would be solved. For example, HammerDB could authenticate to an Azure SQL logical server using a managed identity assigned to an Azure VM where HammerDB is running.

Describe alternatives you've considered
Temporarily enabling SQL authentication on the Azure SQL logical server may be a workaround in some cases. However, this is insecure because the password is stored in clear text in the TCL file.

Additional context
Azure SQL AAD documentation.

@sm-shaw
Copy link
Contributor

sm-shaw commented Feb 5, 2024

This is a good suggestion and definitely something we should do. However, I do not directly have access to an Azure environment to test and therefore to add the functionality will either need a pull request to add what is needed or sufficient guidance to create a branch with what is needed that can then be tested.

So it is not entirely clear whether we can add this by adding additional options to the connect string and a GUI checkbox / CLI option, or whether we need to modify the ODBC interface itself. As an example:

  1. Adding additional options to the connect string was done here to add the encryption and trust certificate options when this changed with ODBC driver 18 Add SQL Server encrypt connection and trust certificate #406 (so this didn't need to change any of the compiled packages). This can provide a template for adding additional connect string options.
  2. Modifying the ODBC interface itself is something we need to do here Extend tdbc::odbc interface to add BCP commands for SQL Server schema builds #594. So for this the bcp functionality has been added by calling the external bcp utility however a much better approach would be to extend the driver so that we can call the ODBC inbuilt bcp commands. This is currently pending.

Also, the connections for both Windows and Linux are different (as we don't have the windows authentication option on Linux) so we would need to know whether this is just something that would be added for a Windows client.

@sm-shaw sm-shaw added enhancement New feature or request database Database specific labels Feb 5, 2024
@dimitri-furman
Copy link
Author

It shouldn't be necessary to modify the ODBC interface. Supporting new values in the connection string Authentication keyword is sufficient. There are five possible values, but to start with, it would be sufficient to only support two of them: ActiveDirectoryInteractive and ActiveDirectoryMsi. The former will prompt the user to authenticate when connecting, the latter supports automation by using a managed identity assigned to the VM where HammerDB is running. If using ActiveDirectoryMsi, the UID connection string keyword is set to the managed identity ID value.

ActiveDirectoryInteractive is Windows only. ActiveDirectoryMsi is both Windows and Linux.

Aside: There is a now a free Azure SQL option for anyone with an Azure subscription. A free Azure account is available as well. This may be suitable for testing.

@sm-shaw
Copy link
Contributor

sm-shaw commented Feb 7, 2024

You should be able to experiment by directly editing the connect_string proc in the Script Editor in the GUI and provide guidance for the correct connect string that is needed.

So I assume we add an "entra" option in addition to Windows and SQL authentication.
Also I assume ActiveDirectoryInteractive with a prompt is Windows GUI only, whereas ActiveDirectoryMsi will also work with CLI?

If so then what we need is something as below and what the correct connect string would look like and also if this is compatible with the azure, encrypt and trust_cert options being appended.

If you can provide this adding to GUI and CLI options to generate this connect string will be straightforward. Unfortunately, I don't have the bandwidth for setting up an Azure account at present so guidance on what will work will be key.

image

Text is below, just cut and paste into the GUI and then modify the connect string to what works.

proc connect_string { server port odbc_driver authentication uid pwd tcp azure db encrypt trust_cert} {
#set to test
set authentication entra
    if { $tcp eq "true" } { set server tcp:$server,$port }
    if {[ string toupper $authentication ] eq "WINDOWS" } {
        set connection "DRIVER=$odbc_driver;SERVER=$server;TRUSTED_CONNECTION=YES"
    } else {
        if {[ string toupper $authentication ] eq "SQL" } {
            set connection "DRIVER=$odbc_driver;SERVER=$server;UID=$uid;PWD=$pwd"
     } else {
        if {[ string toupper $authentication ] eq "ENTRA" } {
            set connection "DRIVER=$odbc_driver;??"
        } else {
            puts stderr "Error: neither WINDOWS, ENTRA or SQL Authentication has been specified"
            set connection "DRIVER=$odbc_driver;SERVER=$server"
        }
    }
}
    if { $azure eq "true" } { append connection ";" "DATABASE=$db" }
    if { $encrypt eq "true" } { append connection ";" "ENCRYPT=yes" } else { append connection ";" "ENCRYPT=no" }
    if { $trust_cert eq "true" } { append connection ";" "TRUSTSERVERCERTIFICATE=yes" }
    return $connection
}

@dimitri-furman
Copy link
Author

Thanks for the guidance, very helpful.

Interactive auth in the GUI works:

image

The sessions on the server connected using my Entra account:

image

I added SERVER=$server;AUTHENTICATION=ActiveDirectoryInteractive to the connection string.

MSI auth in the GUI works as well:
image

image

I added SERVER=$server;AUTHENTICATION=ActiveDirectoryMsi;UID=<msi-object-id>

I see no reason why it wouldn't work in CLI though I am not fully certain how to test that.

Yes, this is compatible with encrypt and trust_cert. I tested with setting each to true and false.

Entra authentication only works with Azure SQL and with SQL Server 2022 (if configured). If azure is false, and if SQL Server version is older than 2022, or if it's 2022 but Entra auth is not configured, then this will fail to connect. But that is expected and the error will be self-explanatory, so I don't think it requires any special handling.

@sm-shaw
Copy link
Contributor

sm-shaw commented Feb 8, 2024

OK, this is great feedback.

So I am thinking that we add another radio button under Authentication to the existing Windows Authentication and SQL Server Authentication with the heading Entra Authentication and if this radio button is selected then we have an additional field of Entra UID (greyed out if not selected). (This will be added to the XML/SQLite config under an new tag e.g. mssqls_entra_uid) By default, in this field we will have "Interactive" so if this is selected it will complete the connection string with:

AUTHENTICATION=ActiveDirectoryInteractive

if this field is not the string "Interactive" it will use:

AUTHENTICATION=ActiveDirectoryMsi;UID=<msi-object-id>

where msi-object-id is whatever is entered in the field instead of Interactive.

Can this UID have special characters that will need escaping like a password?

All the Azure checkbox does is append the string DATABASE=database as previously it used the "use database" command which is not permitted in Azure so we won't do anything with the authentication with this.

Also, if the UID can be used on Linux we will leave the functionality the same on both OS's.

if this sounds like a good approach then we will aim to get this added as the final feature for the next release v4.10 after completing the schema check pull request.

@dimitri-furman
Copy link
Author

I think that will work, but I'd like to propose a slight variation:

an additional field of Entra UID

I suggest calling this MSI Object ID explicitly because no other UID value will work if AUTHENTICATION=ActiveDirectoryMsi. For the same reason, it would be great if the value in this field could be validated to be a well-formed GUID (I don't think you need to do any escaping here, and unlike a password, this value is not a secret).

I realize that this clashes with your proposal to use Interactive as the default value, but we could treat a blank MSI Object ID field as "interactive".

What do you think?

@sm-shaw
Copy link
Contributor

sm-shaw commented Feb 8, 2024

That sounds a good suggestion, so let's go with blank / empty string for interactive

@sm-shaw
Copy link
Contributor

sm-shaw commented Feb 9, 2024

As a quick update - the XML parser doesn't like empty strings, so where we've encountered this before such as the MySQL/MariaDB socket on Windows we will use the string "null" instead and if "null" we use interactive.

image

Also the following:

	if { ![ regexp {[[:xdigit:]]{8}(-[[:xdigit:]]{4}){3}-[[:xdigit:]]{12}|\ynull\y} $mssqls_msi_object_id ] } {
		tk_messageBox -message "MSI Object ID is not a valid format" 
           }

will then validate the field for a correct object id format or the word "null"

@dimitri-furman
Copy link
Author

Understood.

One other note is that the Entra auth option should only be available if the ODBC Driver 18 or later is used. With the older drivers, subsets of functionality may not be available.

@sm-shaw
Copy link
Contributor

sm-shaw commented Feb 11, 2024

Pull request added #665, including details on how to build HammerDB to test this feature to verify it works as intended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database Database specific enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants