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 auth_gssapi authentication. #577

Closed
wants to merge 3 commits into from

Conversation

vaintroub
Copy link
Contributor

@vaintroub vaintroub commented Oct 31, 2018

This patch adds support for MariaDB's gssapi authentication plugin, passwordless authentication on Windows (in both domain and standalone machine environments), as well as on Linux, in Kerberos realm environment.

More information on authentication plugin https://mariadb.com/kb/en/library/authentication-plugin-gssapi

The patch uses .NET NegotiateStream to authenticate. It uses an auxiliary stream to convert NegotiateStream framing for SPNEGO/NTLM payloads into MySQL packet framing with the same payloads.

Things to note

  • Currently, the client "trusts" service principal name that server sends, and passes it as targetName in NegotiateStream.AuthenticateAsClientAsync. Ideally , to support mutual authentication between client and server , service principal name should be an optional connection parameter. If set, driver would ignore the string sent by the server. I did not add this into the patch, but, if we agree it is a good idea, I can be easily added later.

  • On Linux, NegotiateStream implementation currently only works correctly with service principle names (that's the dotnet implementation of it) . Most importantly , it does not understand UPNs user principle names , like machine$@domain.com, which are typically in use by MariaDB servers running inside Windows AD.

Thus by default, connection from a Linux client to Windows server would fail (.NET runtime turns machine$@domain.com into funny name machine$/domain.com, which KDC won't find).

There is however a simple workaround on the server side, to allow Linux client to connect Windows server, would be to set server parameter --gssapi-principal-name=HOST/fqdn@DOMAIN.COM (host SPN, usually added whenever machine joins the domain).

See https://mariadb.com/kb/en/library/authentication-plugin-gssapi/
for plugin details.

Signed-off-by: Vladislav Vaintroub <wlad@mariadb.com>
Copy link
Member

@bgrainger bgrainger left a comment

Thanks for the contribution!

Overall this looks great; I've added a few notes about improving the robustness of the code, mostly for high-concurrency situations.

I assume you've tested this locally (by setting GSSAPIUser in config.json for the tests)? I think it should be possible to get it automatically tested as part of the Travis CI builds (for the mariadb Docker image); I can take a look at that after this is merged.

Ideally , to support mutual authentication between client and server , service principal name should be an optional connection parameter.

Would GSS Service Principal Name (GssServicePrincipalName) be an appropriate connection string option? I see the command-line client uses gssapi-principal-name; I don't know if there's any "prior art" in other connector libraries. I have no objection to your updating this PR to include that setting if you think it would be good to support in the initial implementation.

(Is the verification implementation simply ensuring that AuthGSSAPI.GetServicePrincipalName() returns the same value as was specified in the connection string?)

src/MySqlConnector/Protocol/Serialization/AuthGSSAPI.cs Outdated Show resolved Hide resolved
src/MySqlConnector/Protocol/Serialization/AuthGSSAPI.cs Outdated Show resolved Hide resolved
src/MySqlConnector/Protocol/Serialization/AuthGSSAPI.cs Outdated Show resolved Hide resolved
src/MySqlConnector/Protocol/Serialization/AuthGSSAPI.cs Outdated Show resolved Hide resolved
src/MySqlConnector/Protocol/Serialization/AuthGSSAPI.cs Outdated Show resolved Hide resolved
m_writePayloadLength = 0;
}

public override void Write(byte[] buffer, int offset, int count) => WriteAsync(buffer, offset, count, m_cancellationToken).Wait();
Copy link
Member

@bgrainger bgrainger Oct 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be .GetAwaiter().GetResult() too.

Copy link
Contributor Author

@vaintroub vaintroub Nov 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed that too.

Signed-off-by: Vladislav Vaintroub <wlad@mariadb.com>
@vaintroub
Copy link
Contributor Author

vaintroub commented Nov 1, 2018

I assume you've tested this locally (by setting GSSAPIUser in config.json for the tests)? I think it should be possible to get it automatically tested as part of the Travis CI builds (for the mariadb Docker image); I can take a look at that after this is merged.

Yes, I tested it this way, but just from Windows. On Linux, when I tried to run SideBySide tests, it complained about missing .NET Frameworks, and I ended up testing manually with a tiny console application, that did a connect and disconnect.

I believe the easiest way to automatically test is on Windows, at least for smoke tests, providing the server and client run on the same machine. If both server and client run under domain user accounts, it will test Kerberos, if the machine is standalone, it would use NTLM. I'm not very familiar with appveyor, but our Java Connector uses it, for test, including automated MSI install https://github.com/MariaDB/mariadb-connector-j/blob/master/appveyor.yml , so maybe it is an option, too.

My own Kerberos setup is rather involved, my KDC is Windows Active Directory domain controller, with a Linux and Windows machines joined into the domain, within in a Virtual Box host-only subnet. (I wanted to test same platform or cross-plattform combinations, therefore I made it complicated).

The single-machine Linux testing, with KDC, server and client on the same box is something I did not do this time. But it looks like other people have useful scripts for that , e.g https://github.com/dotnet/corefx/blob/master/src/System.Net.Security/tests/Scripts/Unix/setup-kdc.sh

Would GSS Service Principal Name (GssServicePrincipalName) be an appropriate connection string option? I see the command-line client uses gssapi-principal-name; I don't know if there's any "prior art" in other connector libraries. I have no objection to your updating this PR to include that setting if you think it would be good to support in the initial implementation.

I'd prefer ServerSPN (Server SPN) , because it is how SQLServer's ODBC and JDBC call it.
Let me submit another PR for this, I need to figure how to test it first;)

(Is the verification implementation simply ensuring that AuthGSSAPI.GetServicePrincipalName() returns the same value as was specified in the connection string?)

I thought of something else than comparing strings. A single service can have multiple SPNs, and currently some of them are better for some clients (I mentioned, currently Linux NegotiateStream does not support machine$@domain.com , but it could use host/machine@DOMAIN.COM instead)

I also believe is that is Kerberos creators assumed that client knows the SPN without any help of the server.

So what I would like to use instead, is - whenever ServerSPN is set, ignore whatever server has sent. Instead, use ServerSPN as targetName in AuthenticateAsClient.

If the SPN is wrong, Negotiate might fallback to NTLM, on Windows anyway. We can check for fallback with NegotiateStream.IsMutuallyAuthenticated == false after AuthenticateAsClient, and throw an exception if this happens.

This would render ServerSPN parameter unusable for standalone, non-domain-joined workstations, but I think it is fine if we document that ServerSPN parameter requires Kerberos.

…name.

This parameter is necessary to support mutual authentication feature of the
Kerberos protocol, i.e client can verify server's identity with it.

The parameter can not be used in NTLM scenarios. If Negotiate protocol
falls back to NTLM, and ServerSPN is set, AuthenticationException will be
thrown, because NTLM can not be used to verify server's identity.

This ServerSPN can also be used in some cases to connect to server, if
server does not report correct SPN to clients.


Signed-off-by: Vladislav Vaintroub <wlad@mariadb.com>
@vaintroub
Copy link
Contributor Author

vaintroub commented Nov 2, 2018

Alright, I added the ServerSPN to the pull request, too.
Now all features that need to be there are there.

@@ -58,6 +58,7 @@ public void Defaults()
#if !BASELINE
Assert.Null(csb.ServerRsaPublicKeyFile);
#endif
Assert.Null(csb.ServerSPN);
Copy link
Member

@bgrainger bgrainger Nov 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests also run against Oracle's Connector/NET (to verify equivalent behaviour) so this line will need to be moved inside the #if !BASELINE block to avoid a compiler error with MySqlData.

@@ -121,6 +122,7 @@ public void ParseConnectionString()
"Port=1234;" +
"protocol=pipe;" +
"pwd=Pass1234;" +
"server spn=mariadb/host.example.com@EXAMPLE.COM;" +
Copy link
Member

@bgrainger bgrainger Nov 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move up to line 111 (to be inside #if !BASELINE).

@@ -168,6 +170,7 @@ public void ParseConnectionString()
Assert.False(csb.Pooling);
Assert.Equal(1234u, csb.Port);
Assert.Equal("db-server", csb.Server);
Assert.Equal("mariadb/host.example.com@EXAMPLE.COM", csb.ServerSPN);
Copy link
Member

@bgrainger bgrainger Nov 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move up to line 159 (to be inside #if !BASELINE).

@vaintroub
Copy link
Contributor Author

vaintroub commented Nov 3, 2018

I see you fixed the failing appveyor in #579 . Thanks for explanation, now I see what #if BASELINE was for.

@bgrainger
Copy link
Member

bgrainger commented Nov 3, 2018

Merged via #579.

@bgrainger bgrainger closed this Nov 3, 2018
@vaintroub
Copy link
Contributor Author

vaintroub commented Nov 3, 2018

Thank you!

@bgrainger
Copy link
Member

bgrainger commented Nov 3, 2018

Shipped in 0.47.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants