MDEV-22221: wolfssl tls13 #1501

grooverdan · 2020-04-12T14:10:40Z

libmariadb has exposes ssl options in non-openssl variants for a while so may as well enable them.

Passes the tests in the bug report:

$ /home/dan/repos/build-mariadb-server-10.4/client/mysql -pbob --host=192.168.178.26 --port 3308  --ssl --tls_version=TLSv1.3
Welcome to the MariaDB monitor.  Commands end with ; or \g.
Your MariaDB connection id is 13
Server version: 10.4.13-MariaDB-log Source distribution

Copyright (c) 2000, 2018, Oracle, MariaDB Corporation Ab and others.

Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.

MariaDB [(none)]> \w
Show warnings disabled.
MariaDB [(none)]> \s
--------------
/home/dan/repos/build-mariadb-server-10.4/client/mysql  Ver 15.1 Distrib 10.4.13-MariaDB, for Linux (x86_64) using readline 5.1

Connection id:		13
Current database:	
Current user:		dan@volution.fritz.box
SSL:			Cipher in use is ECDHE-RSA-AES128-GCM-SHA256
Current pager:		stdout
Using outfile:		''
Using delimiter:	;
Server:			MariaDB
Server version:		10.4.13-MariaDB-log Source distribution
Protocol version:	10
Connection:		192.168.178.26 via TCP/IP
Server characterset:	latin1
Db     characterset:	latin1
Client characterset:	utf8
Conn.  characterset:	utf8
TCP port:		3308
Uptime:			34 min 35 sec

Threads: 8  Questions: 11  Slow queries: 0  Opens: 16  Flush tables: 1  Open tables: 10  Queries per second avg: 0.005
--------------

MariaDB [(none)]> show global variables like 'version_ssl_library';
+---------------------+---------------+
| Variable_name       | Value         |
+---------------------+---------------+
| version_ssl_library | WolfSSL 4.3.0 |
+---------------------+---------------+
1 row in set (0.003 sec)

TODO; need to resolve main.ssl_cipher and main.ssl_8k_key test cases and check FP_MAX_BITS cmake condition. Pushing for build results.

cvicentiu · 2020-04-13T11:39:59Z

@grooverdan Looks promising, need some time to digest this myself. Looks like you might still be working on it. Please mark the PR when it's done so I can do a full review.

Also, please squash everything into one commit before I start to review.
Thanks!

grooverdan · 2020-04-14T01:41:34Z

I'm waiting on the wolfssl upstream to resolve the segfault with tls1.3 and 8k certs (or point to my mistake). Without this its a DoS vulnerability. I suspect nothing more is to be done here. The codebase needs some tls1.3 tests but I'm a bit out of time.

What's with the squash commits requirement? These are logically separated, its not like commits are fixes to earlier works. Given https://mariadb.com/kb/en/using-tlsv13/ I was under the assumption that maybe the tests commits might need a backport and maybe the client one too since libmariadb has had non-openssl support for a while.

vaintroub · 2020-04-14T11:35:12Z

vio/viosslfactories.c

@@ -182,6 +182,13 @@ static int wolfssl_send(WOLFSSL* ssl, char* buf, int sz, void* vio)
 {
  return (int)vio_write((Vio *)vio, (unsigned char*)buf, sz);
 }
+
+#ifndef TLS1_3_VERSION
+#define TLS1_3_VERSION


quite right, will remove.

vaintroub · 2020-04-14T11:35:39Z

vio/viosslfactories.c

+#define TLS1_3_VERSION
+#endif
+#ifndef TLS1_2_VERSION
+#define TLS1_2_VERSION


Do we need that?

though it was a cleaner than defined(TLS1_2_VERSION) || defined(HAVE_WOLFSSL) below however these are correctly defined in extra/wolfssl/wolfssl/wolfssl/openssl/{tls1,ssl}.h. I will remove them.

Also Vlad, was there a reason FP_MAX_BITS to 16k was conditional on WOLFSSL_FASTMATH? Still trying to fix main.ssl_8k_key test for TLSv1.3 per comments from wolfssl upstream. There's probably something else other than just defining it as I'm still getting segfaults.

I forgot most of details, but IIRC, FP_MAX_BITS 16K was to please this unrealistic ssl_8k_key test. Nobody in his right mind would allow keys that big, I guess. I think this fastmath was stack based, and alternative was heap based, but I frankly do not remember very much about that.

Thanks. I'll look more closely at that before I finalise this. Still trying to appease the 8k test in tls1.3, the test that keeps on giving :-)

vaintroub · 2020-05-02T09:53:02Z

mysql-test/main/ssl_cipher.result

@@ -3,15 +3,15 @@ create user ssl_user2@localhost require cipher 'AES256-SHA';
 create user ssl_user3@localhost require cipher 'AES256-SHA' AND SUBJECT '/C=FI/ST=Helsinki/L=Helsinki/O=MariaDB/CN=client';
 create user ssl_user4@localhost require cipher 'AES256-SHA' AND SUBJECT '/C=FI/ST=Helsinki/L=Helsinki/O=MariaDB/CN=client' ISSUER '/CN=cacert/C=FI/ST=Helsinki/L=Helsinki/O=MariaDB';
 create user ssl_user5@localhost require cipher 'AES256-SHA' AND SUBJECT 'xxx';
-connect  con1,localhost,ssl_user1,,,,,SSL-CIPHER=AES256-SHA;
+connect  con1,localhost,ssl_user1,,,,,SSL-CIPHER=AES256-SHA TLS-VERSION=TLSv1.1;


I guess you need TLS-VERSION=TLSv1,TLSv1.1,TLSv1.2 here, and in all other places
this excludes TLSv1.3, this is what you want to do, TLS-VERSION=TLSv1.1 would force 1.1 instead

Or, you can maybe automatically set tls_version to "TLSv1,TLSv1.1,TLSv1.2", whenever SSL-CIPHER is given, this would minimize the patch

I like the behaviour of implicit tls_version when SSL-CIPHER Is given. Good idea. I'll probably take a diversion to MDEV-21965 as it has some implications on what the behaviour here should be.

I'm also the the depths of EVP_CipherFinal_ex differences between openssl and wolfssl in the aes unittest failure on AES-GCM. So much for a quick enablement PR.

vaintroub · 2020-05-02T11:08:24Z

mysql-test/main/ssl_8k_key.test

@@ -2,7 +2,7 @@
 #
 # Bug#29784 YaSSL assertion failure when reading 8k key.
 #
--exec $MYSQL --connect-timeout=180 --ssl --ssl-key=$MYSQL_TEST_DIR/std_data/client-key.pem --ssl-cert=$MYSQL_TEST_DIR/std_data/client-cert.pem -e "SELECT (VARIABLE_VALUE <> '') as have_ssl FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME='Ssl_cipher'" 2>&1
+--exec $MYSQL --connect-timeout=180 --ssl --tls-version=TLSv1.1 --ssl-key=$MYSQL_TEST_DIR/std_data/client-key.pem --ssl-cert=$MYSQL_TEST_DIR/std_data/client-cert.pem -e "SELECT (VARIABLE_VALUE <> '') as have_ssl FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME='Ssl_cipher'" 2>&1


is this --tls-version change needed?

grooverdan · 2021-12-03T04:07:33Z

fix committed by @vaintroub last year.

grooverdan mentioned this pull request Apr 13, 2020

SendTls13CertificateVerify: args is local ifndef WOLFSSL_ASYNC_CRYPT wolfSSL/wolfssl#2901

Closed

vaintroub reviewed Apr 14, 2020

View reviewed changes

dgarske mentioned this pull request Apr 14, 2020

Issue with 8k keys and RSA while testing MariaDB wolfSSL/wolfssl#2904

Closed

grooverdan added 5 commits April 15, 2020 16:11

MDEV-22221: enable tlsv1.3 and AES-GCM ciphers on wolfssl

f32b135

MDEV-22221: enable ssl on all client utilities

b11d6be

MDEV-22221: mysqltest to support TLS-VERSION

ebce05f

MDEV-22221: main.ssl_ciphers valid for TLS!=v1.3

40379af

MDEV-22221: main.ssl_8k_key test, cipher valid for TLS!=v1.3

4ae7c96

grooverdan force-pushed the 10.4-MDEV-22221-wolfssl-tls13 branch from 4f8484d to 4ae7c96 Compare April 15, 2020 06:12

vaintroub reviewed May 2, 2020

View reviewed changes

grooverdan marked this pull request as draft May 29, 2020 02:08

an3l added this to the 10.4 milestone Mar 25, 2021

grooverdan closed this Dec 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MDEV-22221: wolfssl tls13 #1501

MDEV-22221: wolfssl tls13 #1501

grooverdan commented Apr 12, 2020

cvicentiu commented Apr 13, 2020

grooverdan commented Apr 14, 2020

vaintroub Apr 14, 2020

grooverdan Apr 15, 2020

vaintroub Apr 14, 2020

grooverdan Apr 15, 2020

vaintroub Apr 15, 2020

grooverdan Apr 15, 2020

vaintroub May 2, 2020

vaintroub May 2, 2020

grooverdan May 3, 2020

vaintroub May 2, 2020 •

edited

grooverdan commented Dec 3, 2021

MDEV-22221: wolfssl tls13 #1501

MDEV-22221: wolfssl tls13 #1501

Conversation

grooverdan commented Apr 12, 2020

cvicentiu commented Apr 13, 2020

grooverdan commented Apr 14, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vaintroub May 2, 2020 • edited

Choose a reason for hiding this comment

grooverdan commented Dec 3, 2021

vaintroub May 2, 2020 •

edited