Skip to content

Commit

Permalink
[MDEV-33822] Comments about a TLS testing mistake
Browse files Browse the repository at this point in the history
When MTR tests use 'NOSSL' and think they are connecting *with SSL
forbidden*, they may actually connecting *with SSL required*.

This is a garbage-in, garbage-out situation: all of the tests that use
'NOSSL', and all of the results, may be wrong.

Adding a prominent FIXME comment here, and otherwise ignoring because this
is too unrelated to the main point of the changes in this branch.

All new code of the whole pull request, including one or several files
that are either new files or modified ones, are contributed under the
BSD-new license. I am contributing on behalf of my employer Amazon Web
Services, Inc.
  • Loading branch information
dlenski committed Apr 26, 2024
1 parent b1bed1a commit 1d05df4
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 4 deletions.
4 changes: 2 additions & 2 deletions client/mysqltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6015,8 +6015,8 @@ int connect_n_handle_errors(struct st_command *command,
enum use_ssl
{
USE_SSL_FORBIDDEN = -1,
USE_SSL_IF_POSSIBLE,
USE_SSL_REQUIRED
USE_SSL_IF_POSSIBLE = 0,
USE_SSL_REQUIRED = 1
};

void do_connect(struct st_command *command)
Expand Down
16 changes: 14 additions & 2 deletions include/sslopt-vars.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,21 @@ SSL_STATIC char *opt_ssl_fp = 0;
SSL_STATIC char *opt_ssl_fplist = 0;
SSL_STATIC my_bool opt_ssl_verify_server_cert= 2;

/* FIXME: there's a major TLS-related hole here. This macro treats
* USE_SSL_FORBIDDEN (-1) identically to USE_SSL_REQUIRED (1). See
* 'enum use_ssl' in mysqltest.cc for their definitions.
*
* This means that, among other things, when MTR tests THINK they are
* connecting WITHOUT SSL, they may actually be connecting with
* obligatory SSL.
*
* Tests that use 'NOSSL' are NOT TESTING WHAT THEY INTEND TO TEST.
*/

#define SET_SSL_OPTS(M) \
do { \
if (opt_use_ssl) \
/* if (opt_use_ssl == -1) {} else */ /* SSL forbidden */ \
if (opt_use_ssl) /* SSL required */ \
{ \
mysql_ssl_set((M), opt_ssl_key, opt_ssl_cert, opt_ssl_ca, \
opt_ssl_capath, opt_ssl_cipher); \
Expand All @@ -48,7 +60,7 @@ SSL_STATIC my_bool opt_ssl_verify_server_cert= 2;
mysql_options((M), MARIADB_OPT_TLS_PEER_FP, opt_ssl_fp); \
mysql_options((M), MARIADB_OPT_TLS_PEER_FP_LIST, opt_ssl_fplist); \
} \
else \
else /* SSL if available */ \
opt_ssl_verify_server_cert= 0; \
mysql_options((M),MYSQL_OPT_SSL_VERIFY_SERVER_CERT, \
&opt_ssl_verify_server_cert); \
Expand Down

0 comments on commit 1d05df4

Please sign in to comment.