Skip to content

Commit

Permalink
WIP: Improve SSL settings, reflect libmysqlclient.so changes for BACK…
Browse files Browse the repository at this point in the history
…RONYM and Riddle vulnerabilities
  • Loading branch information
pali committed Apr 5, 2017
1 parent 3ee67c6 commit ee27812
Show file tree
Hide file tree
Showing 6 changed files with 222 additions and 39 deletions.
14 changes: 14 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ matrix:
env: DB=MySQL VERSION=5.5.8
- perl: "5.20"
env: DB=MySQL VERSION=5.5.47
- perl: "5.20"
env: DB=MySQL VERSION=5.5.49
- perl: "5.20"
env: DB=MySQL VERSION=5.5.54
- perl: "5.20"
Expand All @@ -47,24 +49,36 @@ matrix:
env: DB=MySQL VERSION=5.6.35
- perl: "5.20"
env: DB=MySQL VERSION=5.7.8-rc
- perl: "5.20"
env: DB=MySQL VERSION=5.7.11
- perl: "5.20"
env: DB=MySQL VERSION=5.7.12
- perl: "5.20"
env: DB=MySQL VERSION=5.7.17
- perl: "5.20"
env: DB=MySQL VERSION=8.0.0-dmr
- perl: "5.20"
env: DB=MariaDB VERSION=5.5.40
- perl: "5.20"
env: DB=MariaDB VERSION=5.5.44
- perl: "5.20"
env: DB=MariaDB VERSION=5.5.47
- perl: "5.20"
env: DB=MariaDB VERSION=5.5.54
- perl: "5.20"
env: DB=MariaDB VERSION=10.0.14
- perl: "5.20"
env: DB=MariaDB VERSION=10.0.20
- perl: "5.20"
env: DB=MariaDB VERSION=10.0.23
- perl: "5.20"
env: DB=MariaDB VERSION=10.0.29
- perl: "5.20"
env: DB=MariaDB VERSION=10.1.2
- perl: "5.20"
env: DB=MariaDB VERSION=10.1.8
- perl: "5.20"
env: DB=MariaDB VERSION=10.1.11
- perl: "5.20"
env: DB=MariaDB VERSION=10.1.20
- perl: "5.20"
Expand Down
2 changes: 2 additions & 0 deletions MANIFEST
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ t/88async-multi-stmts.t
t/89async-method-check.t
t/90utf8_params.t
t/91errcheck.t
t/92ssl_optional.t
t/92ssl_riddle_vulnerability.t
t/99_bug_server_prepare_blob_null.t
t/cve-2017-3302.t
t/lib.pl
Expand Down
133 changes: 104 additions & 29 deletions dbdimp.c
Original file line number Diff line number Diff line change
Expand Up @@ -1669,6 +1669,12 @@ void do_warn(SV* h, int rc, char* what)
} \
}

static void set_ssl_error(MYSQL *sock, const char *error)
{
sock->net.last_errno = CR_SSL_CONNECTION_ERROR;
strcpy(sock->net.sqlstate, "HY000");
my_snprintf(sock->net.last_error, sizeof(sock->net.last_error)-1, "SSL connection error: %-.100s", error);
}

/***************************************************************************
*
Expand Down Expand Up @@ -2059,25 +2065,34 @@ MYSQL *mysql_dr_connect(
(SvTRUE(*svp) ? "utf8" : "latin1"));
}

if ((svp = hv_fetch(hv, "mysql_ssl", 9, FALSE)) && *svp && SvTRUE(*svp))
{
#if defined(DBD_MYSQL_WITH_SSL) && !defined(DBD_MYSQL_EMBEDDED) && \
(defined(CLIENT_SSL) || (MYSQL_VERSION_ID >= 40000))
if ((svp = hv_fetch(hv, "mysql_ssl", 9, FALSE)) && *svp)
{
if (SvTRUE(*svp))
{
char *client_key = NULL;
char *client_cert = NULL;
char *ca_file = NULL;
char *ca_path = NULL;
char *cipher = NULL;
STRLEN lna;
#ifdef MYSQL_SSL_MODE
unsigned int ssl_mode = SSL_MODE_PREFERRED;
#endif
unsigned int ssl_mode;
my_bool ssl_enforce = 1;
my_bool ssl_verify = 0;
my_bool ssl_verify_set = 0;

/* Verify if the hostname we connect to matches the hostname in the certificate */
my_bool ssl_verify_true = 0;
if ((svp = hv_fetch(hv, "mysql_ssl_verify_server_cert", 28, FALSE)) && *svp)
ssl_verify_true = SvTRUE(*svp);
if ((svp = hv_fetch(hv, "mysql_ssl_verify_server_cert", 28, FALSE)) && *svp) {
#if defined(HAVE_SSL_VERIFY) || defined(HAVE_SSL_MODE)
ssl_verify = SvTRUE(*svp);
ssl_verify_set = 1;
#else
set_ssl_error(sock, "mysql_ssl_verify_server_cert=1 is not supported");
return NULL;
#endif
}

if ((svp = hv_fetch(hv, "mysql_ssl_optional", 18, FALSE)) && *svp)
ssl_enforce = !SvTRUE(*svp);

if ((svp = hv_fetch(hv, "mysql_ssl_client_key", 20, FALSE)) && *svp)
client_key = SvPV(*svp, lna);
Expand All @@ -2100,35 +2115,95 @@ MYSQL *mysql_dr_connect(

mysql_ssl_set(sock, client_key, client_cert, ca_file,
ca_path, cipher);
#ifdef MYSQL_SSL_MODE
if (ssl_verify_true)

if (ssl_verify && !(ca_file || ca_path)) {
set_ssl_error(sock, "mysql_ssl_verify_server_cert=1 is not supported without mysql_ssl_ca_file or mysql_ssl_ca_path");
return NULL;
}

#ifdef HAVE_SSL_MODE

if (!ssl_enforce)
ssl_mode = SSL_MODE_PREFERRED;
else if (ssl_verify)
ssl_mode = SSL_MODE_VERIFY_IDENTITY;
else if (ca_file)
else if (ca_file || ca_path)
ssl_mode = SSL_MODE_VERIFY_CA;
else if (ca_path)
ssl_mode = SSL_MODE_VERIFY_CA;
mysql_options(sock, MYSQL_OPT_SSL_MODE, &ssl_mode);
#elif MYSQL_VERSION_ID >= SSL_VERIFY_VERSION && MYSQL_VERSION_ID <= SSL_LAST_VERIFY_VERSION || MYSQL_VERSION_ID >= MARIADB_VERSION_10
mysql_options(sock, MYSQL_OPT_SSL_VERIFY_SERVER_CERT, &ssl_verify_true);
#if MYSQL_VERSION_ID >= SSL_ENFORCE_VERSION && MYSQL_VERSION_ID <= SSL_LAST_ENFORCE_VERSION
/* Only needed for Oracle MySQL 5.7 if MYSQL_OPT_SSL_MODE is not available */
mysql_options(sock, MYSQL_OPT_SSL_ENFORCE, &ssl_verify_true);
#endif
else
ssl_mode = SSL_MODE_REQUIRED;
if (mysql_options(sock, MYSQL_OPT_SSL_MODE, &ssl_mode) != 0) {
set_ssl_error(sock, "Setting SSL mode failed");
return NULL;
}

#else

if (ssl_enforce) {
#if defined(HAVE_SSL_MODE_ONLY_REQUIRED)
ssl_mode = SSL_MODE_REQUIRED;
if (mysql_options(sock, MYSQL_OPT_SSL_MODE, &ssl_mode) != 0) {
set_ssl_error(sock, "Enforcing SSL encryption is not supported");
return NULL;
}
#elif defined(HAVE_SSL_ENFORCE)
if (mysql_options(sock, MYSQL_OPT_SSL_ENFORCE, &ssl_enforce) != 0) {
set_ssl_error(sock, "Enforcing SSL encryption is not supported");
return NULL;
}
#elif defined(HAVE_SSL_VERIFY)
if (!ssl_verify_also_enforce_ssl()) {
set_ssl_error(sock, "Enforcing SSL encryption is not supported");
return NULL;
}
if (ssl_verify_set && !ssl_verify) {
set_ssl_error(sock, "Enforcing SSL encryption with mysql_ssl_verify_server_cert=0 is not supported");
return NULL;
}
ssl_verify = 1;
#else
set_ssl_error(sock, "Enforcing SSL encryption is not supported");
return NULL;
#endif
}

#ifdef HAVE_SSL_VERIFY
if (!ssl_enforce && ssl_verify && ssl_verify_also_enforce_ssl()) {
set_ssl_error(sock, "mysql_ssl_optional=0 with mysql_ssl_verify_server_cert=1 is not supported");
return NULL;
}
#endif

if (ssl_verify) {
if (!ssl_verify_usable() && ssl_enforce && ssl_verify_set) {
set_ssl_error(sock, "mysql_ssl_verify_server_cert=1 is broken by current version of MySQL client");
return NULL;
}
#ifdef HAVE_SSL_VERIFY
if (mysql_options(sock, MYSQL_OPT_SSL_VERIFY_SERVER_CERT, &ssl_verify) != 0) {
set_ssl_error(sock, "mysql_ssl_verify_server_cert=1 is not supported");
return NULL;
}
#else
set_ssl_error(sock, "mysql_ssl_verify_server_cert=1 is not supported");
return NULL;
#endif
}

#endif

client_flag |= CLIENT_SSL;
#else
croak("Can't enable strict certificate checks");
set_ssl_error(sock, "mysql_ssl=1 is not supported");
return NULL;
#endif
client_flag |= CLIENT_SSL;
#ifdef MYSQL_SSL_MODE
}
else
else
{
/* mysql_ssl=0 */
#ifdef HAVE_SSL_MODE
unsigned int ssl_mode = SSL_MODE_DISABLED;
mysql_options(sock, MYSQL_OPT_SSL_MODE, &ssl_mode);
#endif
}
}
#endif
#if (MYSQL_VERSION_ID >= 32349)
/*
* MySQL 3.23.49 disables LOAD DATA LOCAL by default. Use
Expand Down
58 changes: 48 additions & 10 deletions dbdimp.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,21 +63,11 @@
#define LIMIT_PLACEHOLDER_VERSION 50007
#define GEO_DATATYPE_VERSION 50007
#define NEW_DATATYPE_VERSION 50003
#define SSL_VERIFY_VERSION 50023
#define SSL_LAST_VERIFY_VERSION 50799
#define SSL_ENFORCE_VERSION 50703
#define SSL_LAST_ENFORCE_VERSION 50799
#define MYSQL_VERSION_5_0 50001
#define MARIADB_VERSION_10 100000
/* This is to avoid the ugly #ifdef mess in dbdimp.c */
#if MYSQL_VERSION_ID < SQL_STATE_VERSION
#define mysql_sqlstate(svsock) (NULL)
#endif
#if MYSQL_VERSION_ID > 50710 && MYSQL_VERSION_ID < MARIADB_VERSION_10
#if MYSQL_VERSION_ID != 60000 /* MySQL Connector/C 6.0 */
#define MYSQL_SSL_MODE
#endif
#endif
/*
* This is the versions of libmysql that supports MySQL Fabric.
*/
Expand All @@ -101,6 +91,54 @@
#define true 1
#define false 0

/*
* Check which SSL settings are supported by API at compile time
*/

/* Use mysql_options with MYSQL_OPT_SSL_VERIFY_SERVER_CERT */
#if ((MYSQL_VERSION_ID >= 50023 && MYSQL_VERSION_ID < 50100) || MYSQL_VERSION_ID >= 50111) && (MYSQL_VERSION_ID < 80000 || defined(MARIADB_BASE_VERSION))
#define HAVE_SSL_VERIFY
#endif

/* Use mysql_options with MYSQL_OPT_SSL_ENFORCE */
#if !defined(MARIADB_BASE_VERSION) && MYSQL_VERSION_ID >= 50703 && MYSQL_VERSION_ID < 80000 && MYSQL_VERSION_ID != 60000
#define HAVE_SSL_ENFORCE
#endif

/* Use mysql_options with MYSQL_OPT_SSL_MODE */
#if !defined(MARIADB_BASE_VERSION) && MYSQL_VERSION_ID >= 50711 && MYSQL_VERSION_ID != 60000
#define HAVE_SSL_MODE
#endif

/* Use mysql_options with MYSQL_OPT_SSL_MODE, but only SSL_MODE_REQUIRED is supported */
#if !defined(MARIADB_BASE_VERSION) && ((MYSQL_VERSION_ID >= 50636 && MYSQL_VERSION_ID < 50700) || (MYSQL_VERSION_ID >= 50555 && MYSQL_VERSION_ID < 50600))
#define HAVE_SSL_MODE_ONLY_REQUIRED
#endif

/*
* Check which SSL settings are supported by API at runtime
*/

/* MYSQL_OPT_SSL_VERIFY_SERVER_CERT automatically enforce SSL mode */
PERL_STATIC_INLINE bool ssl_verify_also_enforce_ssl(void) {
#ifdef MARIADB_BASE_VERSION
my_ulonglong version = mysql_get_client_version();
return ((version >= 50544 && version < 50600) || (version >= 100020 && version < 100100) || version >= 100106);
#else
return false;
#endif
}

/* MYSQL_OPT_SSL_VERIFY_SERVER_CERT is not vulnerable (CVE-2016-2047) and can be used */
PERL_STATIC_INLINE bool ssl_verify_usable(void) {
my_ulonglong version = mysql_get_client_version();
#ifdef MARIADB_BASE_VERSION
return ((version >= 50547 && version < 50600) || (version >= 100023 && version < 100100) || version >= 100110);
#else
return ((version >= 50549 && version < 50600) || (version >= 50630 && version < 50700) || version >= 50712);
#endif
}

/*
* The following are return codes passed in $h->err in case of
* errors by DBD::mysql.
Expand Down
22 changes: 22 additions & 0 deletions t/92ssl_optional.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
use strict;
use warnings;

use Test::More;
use DBI;
use Encode;

use vars qw($test_dsn $test_user $test_password);
require "t/lib.pl";

my $dbh = DbiTestConnect($test_dsn, $test_user, $test_password, { PrintError => 0, RaiseError => 1 });
my $have_ssl = eval { $dbh->selectrow_hashref("SHOW VARIABLES WHERE Variable_name = 'have_ssl'") };
$dbh->disconnect();
plan skip_all => 'Server supports SSL connections, cannot test downgrade to plain text' if $have_ssl and $have_ssl->{Value} eq 'YES';

plan tests => 2;

$dbh = DBI->connect($test_dsn, $test_user, $test_password, { PrintError => 1, RaiseError => 0, mysql_ssl => 1, mysql_ssl_optional => 1 });
ok(defined $dbh, 'DBD::mysql supports mysql_ssl_optional=1 and connect via plain text protocol when SSL is not supported by server') or diag('Error code: ' . ($DBI::err || 'none') . "\n" . 'Error message: ' . ($DBI::errstr || 'unknown'));

$dbh = DBI->connect($test_dsn, $test_user, $test_password, { PrintError => 1, RaiseError => 0, mysql_ssl => 1, mysql_ssl_optional => 1, mysql_ssl_ca_file => "/dev/null" });
ok(defined $dbh, 'DBD::mysql supports mysql_ssl_optional=1 and connect via plain text protocol when SSL is not supported by server even with mysql_ssl_ca_file') or diag('Error code: ' . ($DBI::err || 'none') . "\n" . 'Error message: ' . ($DBI::errstr || 'unknown'));
32 changes: 32 additions & 0 deletions t/92ssl_riddle_vulnerability.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
use strict;
use warnings;

use Test::More;
use DBI;
use Encode;

use vars qw($test_dsn $test_user $test_password);
require "t/lib.pl";

my $dbh = DbiTestConnect($test_dsn, $test_user, $test_password, { PrintError => 0, RaiseError => 1 });
my $have_ssl = eval { $dbh->selectrow_hashref("SHOW VARIABLES WHERE Variable_name = 'have_ssl'") };
$dbh->disconnect();
plan skip_all => 'Server supports SSL connections, cannot test false-positive enforcement' if $have_ssl and $have_ssl->{Value} eq 'YES';

plan tests => 8;

$dbh = DBI->connect($test_dsn, $test_user, $test_password, { PrintError => 0, RaiseError => 0, mysql_ssl => 1 });
ok(!defined $dbh, 'DBD::mysql refused connection to non-SSL server with mysql_ssl=1 and correct user and password');
is($DBI::err, 2026, 'DBD::mysql error message is SSL related') or diag('Error message: ' . ($DBI::errstr || 'unknown'));

$dbh = DBI->connect($test_dsn, $test_user, $test_password, { PrintError => 0, RaiseError => 0, mysql_ssl => 1, mysql_ssl_verify_server_cert => 1, mysql_ssl_ca_file => "" });
ok(!defined $dbh, 'DBD::mysql refused connection to non-SSL server with mysql_ssl=1, mysql_ssl_verify_server_cert=1 and correct user and password');
is($DBI::err, 2026, 'DBD::mysql error message is SSL related') or diag('Error message: ' . ($DBI::errstr || 'unknown'));

$dbh = DBI->connect($test_dsn, '4yZ73s9qeECdWi', '64heUGwAsVoNqo', { PrintError => 0, RaiseError => 0, mysql_ssl => 1 });
ok(!defined $dbh, 'DBD::mysql refused connection to non-SSL server with mysql_ssl=1 and incorrect user and password');
is($DBI::err, 2026, 'DBD::mysql error message is SSL related') or diag('Error message: ' . ($DBI::errstr || 'unknown'));

$dbh = DBI->connect($test_dsn, '4yZ73s9qeECdWi', '64heUGwAsVoNqo', { PrintError => 0, RaiseError => 0, mysql_ssl => 1, mysql_ssl_verify_server_cert => 1, mysql_ssl_ca_file => "" });
ok(!defined $dbh, 'DBD::mysql refused connection to non-SSL server with mysql_ssl=1, mysql_ssl_verify_server_cert=1 and incorrect user and password');
is($DBI::err, 2026, 'DBD::mysql error message is SSL related') or diag('Error message: ' . ($DBI::errstr || 'unknown'));

0 comments on commit ee27812

Please sign in to comment.