[Maria-developers] MDEV-4691 MariaDB Kerberos/GSSAPI

Hi, Wlad! Here's a review, below. Just a few comments or questions. Great to have it done finally! :)

diff --git a/plugin/auth_gssapi/CMakeLists.txt b/plugin/auth_gssapi/CMakeLists.txt new file mode 100644 index 0000000..e20be70 --- /dev/null +++ b/plugin/auth_gssapi/CMakeLists.txt @@ -0,0 +1,33 @@ +IF (WIN32) + SET(USE_SSPI 1) +ENDIF() + +IF(USE_SSPI) + SET(GSSAPI_LIBRARIES secur32) + ADD_DEFINITIONS(-DPLUGIN_SSPI) + SET(GSSAPI_CLIENT sspi_client.cc) + SET(GSSAPI_SERVER sspi_server.cc) + SET(GSSAPI_ERRMSG sspi_errmsg.cc) +ELSE() + SET(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${CMAKE_CURRENT_SOURCE_DIR}/cmake) + FIND_PACKAGE(GSSAPI) + IF(GSSAPI_FOUND) + INCLUDE_DIRECTORIES(${GSSAPI_INCS}) + ADD_DEFINITIONS(-DPLUGIN_GSSAPI) + SET(GSSAPI_CLIENT gssapi_client.cc) + SET(GSSAPI_SERVER gssapi_server.cc) + SET(GSSAPI_ERRMSG gssapi_errmsg.cc) + ELSE() + # Can't build plugin + RETURN() + ENDIF() +ENDIF () + + +MYSQL_ADD_PLUGIN(auth_gssapi server_plugin.cc ${GSSAPI_SERVER} ${GSSAPI_ERRMSG} + LINK_LIBRARIES ${GSSAPI_LIBS} + MODULE_ONLY)

please, specify a component here and below. Like "COMPONENT Kerberos" (or gssapi, whatever). It would mean that this plugin will be in a separate rpm package - it must be, because it brings additional dependencies that the server itself does not need.

+ +MYSQL_ADD_PLUGIN(auth_gssapi_client client_plugin.cc ${GSSAPI_CLIENT} ${GSSAPI_ERRMSG} + LINK_LIBRARIES ${GSSAPI_LIBS} + MODULE_ONLY)

same here. I'm not sure whether it should be the same component as above, or a different one. also, I'm not sure whether plugin.cmake will do a correct client plugin rpm, if you'll have a different component here.

diff --git a/plugin/auth_gssapi/README.md b/plugin/auth_gssapi/README.md new file mode 100644 index 0000000..f20128f --- /dev/null +++ b/plugin/auth_gssapi/README.md @@ -0,0 +1,129 @@ +# GSSAPI/SSPI authentication for MariaDB + +This article gives instructions on configuring GSSAPI authentication plugin +for MariaDB for passwordless login. + +On Unix systems, GSSAPI is usually synonymous with Kerberos authentication. +Windows has slightly different but very similar API called SSPI, that along with Kerberos, also supports NTLM authentication. + +This plugin includes support for Kerberos on Unix, but also can be used as for Windows authentication with or without domain +environment. + +## Server-side preparations on Unix +To use the plugin, some preparation need to be done on the server side on Unixes. +MariaDB server will read need access to the Kerberos keytab file, that contains service principal name for the MariaDB server.

s/read need/need read/ :)

+ + +If you are using **Unix Kerberos KDC (MIT,Heimdal)** + +- Create service principal using kadmin tool + +``` +kadmin -q "addprinc -randkey mariadb/host.domain.com" +``` + +(replace host.domain.com with fully qualified DNS name for the server host) + +- Export the newly created user to the keytab file + +``` +kadmin -q "ktadd -k /path/to/mariadb.keytab mariadb/host.domain.com" +``` + +More details can be found [here](http://www.microhowto.info/howto/create_a_service_principal_using_mit_kerber...) +and [here](http://www.microhowto.info/howto/add_a_host_or_service_principal_to_a_keytab...) + +If you are using **Windows Active Directory KDC** +you can need to create keytab using ktpass.exe tool on Windows, map principal user to an existing domain user like this + +``` +ktpass.exe /princ mariadb/host.domain.com@DOMAIN.COM /mapuser someuser /pass MyPas$w0rd /out mariadb.keytab /crypto all /ptype KRB5_NT_PRINCIPAL /mapop set +``` + +and then transfer the keytab file to the Unix server. See [Microsoft documentation](https://technet.microsoft.com/en-us/library/cc753771.aspx) for details. + + +## Server side preparations on Windows. +Usually nothing need to be done. MariaDB server should to run on a domain joined machine, either as NetworkService account +(which is default if it runs as service) or run under any other domain account credentials. +Creating service principal is not required here (but you can still do it using [_setspn_](https://technet.microsoft.com/en-us/library/cc731241.aspx) tool) + + +# Installing plugin +- Start the server + +- On Unix, edit my the my.cnf/my.ini configuration file, set the parameter gssapi-keytab-path to point to previously +created keytab path. + +``` + gssapi-keytab-path=/path/to/mariadb.keytab +``` + +- Optionally on Unix, in case the service principal name differs from default mariadb/host.domain.com@REALM, +configure alternative principal name with + +``` + gssapi-principal-name=alternative/principalname@REALM +``` + +- In mysql command line client, execute + +``` + INSTALL SONAME 'auth_gssapi' +``` + +#Creating users + +Now, you can create a user for GSSAPI/SSPI authentication. CREATE USER command, for Kerberos user +would be like this (*long* form, see below for short one) + +``` +CREATE USER usr1 IDENTIFIED WITH gssapi AS 'usr1@EXAMPLE.COM'; +``` + +(replace with real username and realm) + +The part after AS is mechanism specific, and needs to be ``machine\\usr1`` for Windows users identified with NTLM. + +You may also use alternative *short* form of CREATE USER + +``` +CREATE USER usr1 IDENTIFIED WITH gssapi; +``` + +If this syntax is used, realm part is used for comparison

realm part is NOT used for comparison

+thus 'usr1@EXAMPLE.COM', 'usr1@EXAMPLE.CO.UK' and 'mymachine\usr1' will all identify as 'usr1'. + +#Login as GSSAPI user with command line clients + +Using command line client, do + +``` +mysql --plugin-dir=/path/to/plugin-dir -u usr1 +``` + +#Plugin variables +- **gssapi-keytab-path** (Unix only) - Path to the server keytab file +- **gssapi-principal-name** - name of the service principal. +- **gssapi-mech-name** (Windows only) - Name of the SSPI package used by server. Can be either 'Kerberos' or 'Negotiate'. + Defaults to 'Negotiate' (both Kerberos and NTLM users can connect) + Set it to 'Kerberos', to prevent less secure NTLM in domain environments, but leave it as default(Negotiate) + to allow non-domain environment (e.g if server does not run in domain enviroment). + + +#Implementation + +Overview of the protocol between client and server + +1. Server : Construct gssapi-principal-name if not set in my.cnf. On Unixes defaults to hostbased name for service "mariadb". On Windows to user's or machine's domain names. +Acquire credentials for gssapi-principal-name with ```gss_acquire_cred() / AcquireSecurityCredentials()```. +Send packet with principal name and mech ```"gssapi-principal-name\0gssapi-mech-name\0"``` to client ( on Unix, empty string used for gssapi-mech) + +2. Client: execute ```gss_init_sec_context() / InitializeSecurityContext()``` passing gssapi-principal-name / gssapi-mech-name parameters. +Send resulting GSSAPI blob to server. + +3. Server : receive blob from client, execute ```gss_accept_sec_context()/ AcceptSecurityContext()```, send resulting blob back to client + +4. Perform 2. and 3. can until both client and server decide that authentication is done, or until some error occured. If authentication was successful, GSSAPI context (an opaque structure) is generated on both client and server sides. + +5. Server : Client name is extracted from the context, and compared to the name provided by client(with or without realm). If name matches, plugin returns success.

This is a very good documentation. But it also has to be in the KB (perhaps minus with "Implementation" part). You can put it there yourself or ask Ian to do it. There's a feature in KB that allows to "link" a KB page to a "Github flavored markup" document (meaning the markup document is the original and KB is a copy of it). Perhaps it can be useful here.

diff --git a/plugin/auth_gssapi/cmake/FindGSSAPI.cmake b/plugin/auth_gssapi/cmake/FindGSSAPI.cmake new file mode 100644 index 0000000..faee428 --- /dev/null +++ b/plugin/auth_gssapi/cmake/FindGSSAPI.cmake @@ -0,0 +1,78 @@ +# - Try to detect the GSSAPI support +# Once done this will define +# +# GSSAPI_FOUND - system supports GSSAPI +# GSSAPI_INCS - the GSSAPI include directory +# GSSAPI_LIBS - the libraries needed to use GSSAPI +# GSSAPI_FLAVOR - the type of API - MIT or HEIMDAL + +# Copyright (c) 2006, Pino Toscano, +# +# Redistribution and use is allowed according to the terms of the BSD license. +# For details see the accompanying COPYING-CMAKE-SCRIPTS file.

There's no accompanying COPYING-CMAKE-SCRIPTS file, as far as I can see. You either need to add the license in this file or take this COPYING-CMAKE-SCRIPTS too. I'd copied the complete license into this file, I think.

+ + +if(GSSAPI_LIBS AND GSSAPI_FLAVOR) + + # in cache already + set(GSSAPI_FOUND TRUE) + +else(GSSAPI_LIBS AND GSSAPI_FLAVOR) + + find_program(KRB5_CONFIG NAMES krb5-config PATHS + /opt/local/bin + ONLY_CMAKE_FIND_ROOT_PATH # this is required when cross compiling with cmake 2.6 and ignored with cmake 2.4, Alex + ) + mark_as_advanced(KRB5_CONFIG) + + #reset vars + set(GSSAPI_INCS) + set(GSSAPI_LIBS) + set(GSSAPI_FLAVOR) + + if(KRB5_CONFIG) + + set(HAVE_KRB5_GSSAPI TRUE) + exec_program(${KRB5_CONFIG} ARGS --libs gssapi RETURN_VALUE _return_VALUE OUTPUT_VARIABLE GSSAPI_LIBS) + if(_return_VALUE) + message(STATUS "GSSAPI configure check failed.") + set(HAVE_KRB5_GSSAPI FALSE) + endif(_return_VALUE) + + exec_program(${KRB5_CONFIG} ARGS --cflags gssapi RETURN_VALUE _return_VALUE OUTPUT_VARIABLE GSSAPI_INCS) + string(REGEX REPLACE "(\r?\n)+$" "" GSSAPI_INCS "${GSSAPI_INCS}") + string(REGEX REPLACE " *-I" ";" GSSAPI_INCS "${GSSAPI_INCS}") + + exec_program(${KRB5_CONFIG} ARGS --vendor RETURN_VALUE _return_VALUE OUTPUT_VARIABLE gssapi_flavor_tmp) + set(GSSAPI_FLAVOR_MIT) + if(gssapi_flavor_tmp MATCHES ".*Massachusetts.*") + set(GSSAPI_FLAVOR "MIT") + else(gssapi_flavor_tmp MATCHES ".*Massachusetts.*") + set(GSSAPI_FLAVOR "HEIMDAL") + endif(gssapi_flavor_tmp MATCHES ".*Massachusetts.*") + + if(NOT HAVE_KRB5_GSSAPI) + if (gssapi_flavor_tmp MATCHES "Sun Microsystems.*") + message(STATUS "Solaris Kerberos does not have GSSAPI; this is normal.") + set(GSSAPI_LIBS) + set(GSSAPI_INCS) + else(gssapi_flavor_tmp MATCHES "Sun Microsystems.*") + message(WARNING "${KRB5_CONFIG} failed unexpectedly.") + endif(gssapi_flavor_tmp MATCHES "Sun Microsystems.*") + endif(NOT HAVE_KRB5_GSSAPI) + + if(GSSAPI_LIBS) # GSSAPI_INCS can be also empty, so don't rely on that + set(GSSAPI_FOUND TRUE CACHE STRING "") + message(STATUS "Found GSSAPI: ${GSSAPI_LIBS}") + + set(GSSAPI_INCS ${GSSAPI_INCS} CACHE STRING "") + set(GSSAPI_LIBS ${GSSAPI_LIBS} CACHE STRING "") + set(GSSAPI_FLAVOR ${GSSAPI_FLAVOR} CACHE STRING "") + + mark_as_advanced(GSSAPI_INCS GSSAPI_LIBS GSSAPI_FLAVOR) + + endif(GSSAPI_LIBS) + + endif(KRB5_CONFIG) + +endif(GSSAPI_LIBS AND GSSAPI_FLAVOR) diff --git a/plugin/auth_gssapi/mysql-test/auth_gssapi/suite.pm b/plugin/auth_gssapi/mysql-test/auth_gssapi/suite.pm new file mode 100644 index 0000000..3ffc6f1 --- /dev/null +++ b/plugin/auth_gssapi/mysql-test/auth_gssapi/suite.pm @@ -0,0 +1,47 @@ +package My::Suite::AuthGSSAPI; + +@ISA = qw(My::Suite); + +return "No AUTH_GSSAPI plugin" unless $ENV{AUTH_GSSAPI_SO}; + +return "Not run for embedded server" if $::opt_embedded_server; + +# Following environment variables may need to be set +if ($^O eq "MSWin32") +{ + chomp(my $whoami =`whoami /UPN 2>NUL` || `whoami`);

don't you need to check whether the command worked and whether it returned something valid?

+ my $fullname = $whoami; + $fullname =~ s/\\/\\\\/; # SQL escaping for backslash + $ENV{'GSSAPI_FULLNAME'} = $fullname; + $ENV{'GSSAPI_SHORTNAME'} = $ENV{'USERNAME'}; +} +else +{ + if (!$ENV{'GSSAPI_FULLNAME'}) + { + my $s = `klist |grep 'Default principal: '`; + if ($s) + { + chomp($s); + my $fullname = substr($s,19); + $ENV{'GSSAPI_FULLNAME'} = $fullname; + } + } + $ENV{'GSSAPI_SHORTNAME'} = (split /@/, $ENV{'GSSAPI_FULLNAME'}) [0]; +} + + +if (!$ENV{'GSSAPI_FULLNAME'} || !$ENV{'GSSAPI_SHORTNAME'}) +{ + return "Environment variable GSSAPI_SHORTNAME and GSSAPI_FULLNAME need to be set" +} + +foreach $var ('GSSAPI_SHORTNAME','GSSAPI_FULLNAME','GSSAPI_KEYTAB_PATH','GSSAPI_PRINCIPAL_NAME') +{ + print "$var=$ENV{$var}\n";

Really? It would make mtr runs a bit too verbose. at least put it inside if ($::opt_verbose) { ... }

+} + +sub is_default { 1 } + +bless { }; + diff --git a/plugin/auth_gssapi/mysql-test/auth_gssapi/suite.opt b/plugin/auth_gssapi/mysql-test/auth_gssapi/suite.opt new file mode 100644 index 0000000..3077d70 --- /dev/null +++ b/plugin/auth_gssapi/mysql-test/auth_gssapi/suite.opt @@ -0,0 +1 @@ +--loose-gssapi-keytab-path=$GSSAPI_KEYTAB_PATH --loose-gssapi-principal-name=$GSSAPI_PRINCIPAL_NAME

1. who sets GSSAPI_KEYTAB_PATH and GSSAPI_PRINCIPAL_NAME? 2. if the user, do you need to check in suite.pm whether they're set?

diff --git a/plugin/auth_gssapi/mysql-test/auth_gssapi/basic.result b/plugin/auth_gssapi/mysql-test/auth_gssapi/basic.result new file mode 100644 index 0000000..a859ce5 --- /dev/null +++ b/plugin/auth_gssapi/mysql-test/auth_gssapi/basic.result @@ -0,0 +1,18 @@ +INSTALL SONAME 'auth_gssapi'; +CREATE USER GSSAPI_SHORTNAME IDENTIFIED WITH gssapi; +SELECT USER(),CURRENT_USER(); +USER() CURRENT_USER() +GSSAPI_SHORTNAME@localhost GSSAPI_SHORTNAME@% +DROP USER GSSAPI_SHORTNAME; +CREATE USER nosuchuser IDENTIFIED WITH gssapi; +ERROR 28000: GSSAPI name mismatch, requested 'nosuchuser', actual name 'GSSAPI_SHORTNAME'

What does the real unmasked error message look like? Is it something that client can know without connecting to the server? If not - this user-friendly verbose error is an information disclosure security vulnerability

+DROP USER nosuchuser; +CREATE USER usr1 IDENTIFIED WITH gssapi as 'GSSAPI_FULLNAME'; +SELECT USER(),CURRENT_USER(); +USER() CURRENT_USER() +usr1@localhost usr1@% +DROP USER usr1; +CREATE USER nosuchuser IDENTIFIED WITH gssapi AS 'nosuchuser@EXAMPLE.COM'; +ERROR 28000: GSSAPI name mismatch, requested 'nosuchuser@EXAMPLE.COM', actual name 'GSSAPI_FULLNAME' +DROP USER nosuchuser; +UNINSTALL SONAME 'auth_gssapi'; diff --git a/plugin/auth_gssapi/server_plugin.cc b/plugin/auth_gssapi/server_plugin.cc new file mode 100644 index 0000000..64f52a3 --- /dev/null +++ b/plugin/auth_gssapi/server_plugin.cc @@ -0,0 +1,175 @@ +/* Copyright (c) 2015, Shuang Qiu, Robbie Hardwood, + Vladislav Vaintroub & MariaDB Corporation + + All rights reserved. + + Redistribution and use in source and binary forms, with or without + modification, are permitted provided that the following conditions are met: + + 1. Redistributions of source code must retain the above copyright notice, + this list of conditions and the following disclaimer. + + 2. Redistributions in binary form must reproduce the above copyright notice, + this list of conditions and the following disclaimer in the documentation + and/or other materials provided with the distribution. + + THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + POSSIBILITY OF SUCH DAMAGE.

2-clause BSD. Was it Robbie who asked for it?

+*/ + +/** + @file + + GSSAPI authentication plugin, server side +*/ +#include +#include +#include +#include "server_plugin.h" +#include "common.h" + +/* First packet sent from server to client, contains srv_principal_name\0mech\0 */ +static char first_packet[PRINCIPAL_NAME_MAX + MECH_NAME_MAX +2]; +static int first_packet_len; + +/* + Target name in GSSAPI/SSPI , for Kerberos it is service principal name + (often user principal name of the server user will work) +*/ +char *srv_principal_name; +char *srv_keytab_path; +char *srv_mech_name=(char *)""; +unsigned long srv_mech; + +/** + The main server function of the GSSAPI plugin. + */ +static int gssapi_auth(MYSQL_PLUGIN_VIO *vio, MYSQL_SERVER_AUTH_INFO *auth_info) +{ + int use_full_name; + const char *user; + int user_len; + + /* Send first packet with target name and mech name */ + if (vio->write_packet(vio, (unsigned char *)first_packet, first_packet_len)) + { + return CR_ERROR; + } + + /* Figure out whether to use full name (as given in IDENTIFIED AS clause) + * or just short username auth_string + */ + if (auth_info->auth_string_length > 0)

Hmm, I wonder how that can possibly work. auth_info->auth_string can only be set when the user is known. And the user is known only after the first read... ... ok ... got it. the first packet always uses the default, mysql_native_password, plugin so when the server switches to gssapi plugin, the user name is already known. Still, in case we'll have a --default-auth-plugin server option, I'd suggest to check auth_info->auth_string_length only after the first vio->read_packet()

+ { + use_full_name= 1; + user= auth_info->auth_string; + user_len= auth_info->auth_string_length; + } + else + { + use_full_name= 0; + user= auth_info->user_name; + user_len= auth_info->user_name_length; + } + + return auth_server(vio, user, user_len, use_full_name); +} + +static int initialize_plugin(void *unused) +{ + int rc; + rc = plugin_init(); + if (rc) + return rc; + + strcpy(first_packet, srv_principal_name); + strcpy(first_packet + strlen(srv_principal_name) + 1,srv_mech_name); + first_packet_len = strlen(srv_principal_name) + strlen(srv_mech_name) + 2;

in the server code I'd use strmov here, like first_packet_len = strmov(strmov(first_packet, srv_principal_name) + 1, srv_mech_name) - first_packet + 1; (may be with intermediate variables, for readability). I understand that a plugin might not want to do it, to avoid depending on server code outside of the plugin API. But if that's the case, why do you include my_sys.h? And mysqld_error.h?

+ + return 0; +} + +static int deinitialize_plugin(void *unused) +{ + return plugin_deinit(); +} + +/* system variable */ +static MYSQL_SYSVAR_STR(keytab_path, srv_keytab_path, + PLUGIN_VAR_RQCMDARG|PLUGIN_VAR_READONLY, + "Keytab file path (Kerberos)", + NULL, + NULL, + ""); +static MYSQL_SYSVAR_STR(principal_name, srv_principal_name, + PLUGIN_VAR_RQCMDARG|PLUGIN_VAR_READONLY, + "GSSAPI target name - service principal name for Kerberos authentication.",

A bit inconsistent. I'd suggest either to change the first to Keytab file path for Kerberos authentication or the second to GSSAPI target name - service principal name (Kerberos)

+ NULL, + NULL, + ""); +#ifdef PLUGIN_SSPI +static const char* mech_names[] = { + "Kerberos", + "Negotiate", + "", + NULL +}; +static TYPELIB mech_name_typelib = { + array_elements(mech_names) - 1, + "mech_name_typelib", + mech_names, + NULL +}; +static MYSQL_SYSVAR_ENUM(mech_name, srv_mech, + PLUGIN_VAR_RQCMDARG|PLUGIN_VAR_READONLY, + "GSSAPI mechanism : either Kerberos or Negotiate",

You don't need "either Kerberos or Negotiate" here, simply "GSSAPI mechanism" is enough. On --help, my_getopt automatically will add ", one of: Kerberos, Negotiate". Which is consistent with other ENUM options. It only makes sense to list values explicitly, when the list contains additional information, like "GSSAPI mechanism : either Kerberos or Negotiate (automatically choose between Kerberos and NTLM)."

+ NULL, + NULL, + 2,&mech_name_typelib); +#endif + +static struct st_mysql_sys_var *system_variables[]= { + MYSQL_SYSVAR(principal_name), +#ifdef PLUGIN_SSPI + MYSQL_SYSVAR(mech_name), +#endif +#ifdef PLUGIN_GSSAPI + MYSQL_SYSVAR(keytab_path), +#endif + NULL +}; + +/* Register authentication plugin */ +static struct st_mysql_auth server_handler= { + MYSQL_AUTHENTICATION_INTERFACE_VERSION, + "auth_gssapi_client", + gssapi_auth +}; + +maria_declare_plugin(gssapi_server) +{ + MYSQL_AUTHENTICATION_PLUGIN, + &server_handler, + "gssapi", + "Shuang Qiu, Robbie Harwood, Vladislav Vaintroub", + "Plugin for GSSAPI/SSPI based authentication.", + PLUGIN_LICENSE_BSD, + initialize_plugin, + deinitialize_plugin, /* destructor */ + 0x0100, /* version */ + NULL, /* status variables */ + system_variables, /* system variables */ + "1.0", + MariaDB_PLUGIN_MATURITY_EXPERIMENTAL

I'd say BETA

+} +maria_declare_plugin_end;

Regards, Sergei Chief Architect MariaDB and security@mariadb.org -- Vote for my Percona Live 2016 talks: https://www.percona.com/live/data-performance-conference-2016/sessions/maria... https://www.percona.com/live/data-performance-conference-2016/sessions/maria...