Hi, Georg, On Jul 31, Georg Richter wrote:

revision-id: 1287c901 (v3.4.0-5-g1287c901) parent(s): 5386f1a3 author: Georg Richter committer: Georg Richter timestamp: 2024-07-16 13:12:26 +0200 message:

TLS/SSL changes (major rework)

diff --git a/include/ma_common.h b/include/ma_common.h index dfa96621..dc900b0d 100644 --- a/include/ma_common.h +++ b/include/ma_common.h @@ -131,15 +131,9 @@ typedef struct st_mariadb_field_extension MARIADB_CONST_STRING metadata[MARIADB_FIELD_ATTR_LAST+1]; /* 10.5 */ } MA_FIELD_EXTENSION;

-#if defined(HAVE_SCHANNEL) || defined(HAVE_GNUTLS) -#define reset_tls_self_signed_error(mysql) \ - do { \ - free((char*)mysql->net.tls_self_signed_error); \ - mysql->net.tls_self_signed_error= 0; \ - } while(0) -#else -#define reset_tls_self_signed_error(mysql) \ - do { \ - mysql->net.tls_self_signed_error= 0; \ +#ifdef HAVE_TLS +#define reset_tls_error(mysql) \ + do { \ + mysql->net.tls_verify_status= 0; \

you've lost the original error message when you've changed char* pointer to the message to my_bool flag.

} while(0) #endif diff --git a/include/ma_tls.h b/include/ma_tls.h index 23f53560..444ea4aa 100644 --- a/include/ma_tls.h +++ b/include/ma_tls.h @@ -134,6 +141,7 @@ const char *ma_tls_get_cipher(MARIADB_TLS *ssl); hash_type hash_type as defined in ma_hash.h fp buffer for fingerprint fp_len buffer length + my_bool verify_period

I don't see any 'my bool verify_period' in the list of arguments

Returns: actual size of finger print @@ -150,7 +158,7 @@ unsigned int ma_tls_get_finger_print(MARIADB_TLS *ctls, uint hash_type, char *fp int ma_tls_get_protocol_version(MARIADB_TLS *ctls); const char *ma_pvio_tls_get_protocol_version(MARIADB_TLS *ctls); int ma_pvio_tls_get_protocol_version_id(MARIADB_TLS *ctls); -unsigned int ma_tls_get_peer_cert_info(MARIADB_TLS *ctls); +unsigned int ma_tls_get_peer_cert_info(MARIADB_TLS *ctls, unsigned int size);

is this part of the public API?

void ma_tls_set_connection(MYSQL *mysql);

/* Function prototypes */ diff --git a/include/mysql.h b/include/mysql.h index 6c84a462..ba92527d 100644 --- a/include/mysql.h +++ b/include/mysql.h @@ -448,6 +449,16 @@ typedef struct st_mysql_time #define MYSQL_WAIT_EXCEPT 4 #define MYSQL_WAIT_TIMEOUT 8

+#define MARIADB_TLS_VERIFY_OK 0 +#define MARIADB_TLS_VERIFY_TRUST 1 +#define MARIADB_TLS_VERIFY_HOST 2

we've agreed to swap two previous lines

+#define MARIADB_TLS_VERIFY_PERIOD 4 +#define MARIADB_TLS_VERIFY_FINGERPRINT 8

is that right? you want to allow fingerprints even if the cert is expired?

+#define MARIADB_TLS_VERIFY_REVOKED 16 +#define MARIADB_TLS_VERIFY_UNKNOWN 32 +#define MARIADB_TLS_VERIFY_ERROR 128 /* last */ + + typedef struct character_set { unsigned int number; /* character set number */ diff --git a/libmariadb/ma_tls.c b/libmariadb/ma_tls.c index 1bda7dcf..f6ea6661 100644 --- a/libmariadb/ma_tls.c +++ b/libmariadb/ma_tls.c @@ -103,9 +103,54 @@ my_bool ma_pvio_tls_close(MARIADB_TLS *ctls) return ma_tls_close(ctls); }

-int ma_pvio_tls_verify_server_cert(MARIADB_TLS *ctls) +int ma_pvio_tls_verify_server_cert(MARIADB_TLS *ctls, unsigned int flags) { - return ma_tls_verify_server_cert(ctls); + MYSQL *mysql; + int rc; + + if (!ctls || !ctls->pvio || !ctls->pvio->mysql) + return 0; + + mysql= ctls->pvio->mysql; + + /* Skip peer certificate verification */ + if (ctls->pvio->mysql->options.extension->tls_allow_invalid_server_cert) + { + return 0; + } + + rc= ma_tls_verify_server_cert(ctls, flags); + + /* Set error messages */ + if (!mysql->net.last_errno) + { + if (mysql->net.tls_verify_status & MARIADB_TLS_VERIFY_PERIOD) + my_set_error(mysql, CR_SSL_CONNECTION_ERROR, SQLSTATE_UNKNOWN, + ER(CR_SSL_CONNECTION_ERROR), + "Certificate not yet valid or expired"); + else if (mysql->net.tls_verify_status & MARIADB_TLS_VERIFY_FINGERPRINT) + my_set_error(mysql, CR_SSL_CONNECTION_ERROR, SQLSTATE_UNKNOWN, + ER(CR_SSL_CONNECTION_ERROR), + "Fingerprint validation of peer certificate failed"); + else if (mysql->net.tls_verify_status & MARIADB_TLS_VERIFY_REVOKED) + my_set_error(mysql, CR_SSL_CONNECTION_ERROR, SQLSTATE_UNKNOWN, + ER(CR_SSL_CONNECTION_ERROR), + "Certificate revoked"); + else if (mysql->net.tls_verify_status & MARIADB_TLS_VERIFY_HOST) + my_set_error(mysql, CR_SSL_CONNECTION_ERROR, SQLSTATE_UNKNOWN, + ER(CR_SSL_CONNECTION_ERROR), + "Hostname verification failed"); + else if (mysql->net.tls_verify_status & MARIADB_TLS_VERIFY_UNKNOWN) + my_set_error(mysql, CR_SSL_CONNECTION_ERROR, SQLSTATE_UNKNOWN, + ER(CR_SSL_CONNECTION_ERROR), + "Peer certificate verification failed"); + else if (mysql->net.tls_verify_status & MARIADB_TLS_VERIFY_TRUST) + my_set_error(mysql, CR_SSL_CONNECTION_ERROR, SQLSTATE_UNKNOWN, + ER(CR_SSL_CONNECTION_ERROR), + "Peer certificate is not trusted");

let's try to preserve the error as set by the ssl library here

+ } + + return rc; }

const char *ma_pvio_tls_cipher(MARIADB_TLS *ctls) diff --git a/plugins/auth/my_auth.c b/plugins/auth/my_auth.c index faea968c..e7429fae 100644 --- a/plugins/auth/my_auth.c +++ b/plugins/auth/my_auth.c @@ -382,14 +419,30 @@ static int send_client_reply_packet(MCPVIO_EXT *mpvio, errno); goto error; } + mysql->net.tls_verify_status = 0; if (ma_pvio_start_ssl(mysql->net.pvio)) goto error; - if (mysql->net.tls_self_signed_error && - (!mysql->passwd || !mysql->passwd[0] || !hashing(mpvio->plugin))) - { - /* cannot use auth to validate the cert */ - set_error_from_tls_self_signed_error(mysql); - goto error; + + verify_flags= MARIADB_TLS_VERIFY_PERIOD | MARIADB_TLS_VERIFY_REVOKED; + if (have_fingerprint(mysql)) + { + verify_flags|= MARIADB_TLS_VERIFY_FINGERPRINT; + } else { + verify_flags|= MARIADB_TLS_VERIFY_TRUST | MARIADB_TLS_VERIFY_HOST; + } + + if (ma_pvio_tls_verify_server_cert(mysql->net.pvio->ctls, verify_flags) > MARIADB_TLS_VERIFY_OK) + { + if (mysql->net.tls_verify_status > MARIADB_TLS_VERIFY_TRUST || + (mysql->options.ssl_ca || mysql->options.ssl_capath)) + goto error; + + if (is_local_connection(mysql->net.pvio))

I'd say this should be earlier. Like if (!is_local_connection(mysql->net.pvio)) if (have_fingerprint(mysql)) { verify_flags|= MARIADB_TLS_VERIFY_FINGERPRINT; } else { verify_flags|= MARIADB_TLS_VERIFY_TRUST | MARIADB_TLS_VERIFY_HOST; }

+ { + CLEAR_CLIENT_ERROR(mysql); + } + else if (!password_and_hashing(mysql, mpvio->plugin)) + goto error; } } #endif /* HAVE_TLS */ @@ -769,8 +822,14 @@ retry: auth_plugin= &dummy_fallback_client_plugin;

/* can we use this plugin with this tls server cert ? */ - if (mysql->net.tls_self_signed_error && !hashing(auth_plugin)) - return set_error_from_tls_self_signed_error(mysql); + if ((mysql->net.tls_verify_status & MARIADB_TLS_VERIFY_TRUST) && + !password_and_hashing(mysql, auth_plugin)) + { + my_set_error(mysql, CR_SSL_CONNECTION_ERROR, SQLSTATE_UNKNOWN, + ER(CR_SSL_CONNECTION_ERROR), + "Certificate verification failure: The certificate is NOT trusted.");

already commented about using library error message

+ return 1; + } goto retry; } /* @@ -782,7 +841,9 @@ retry: if (ma_read_ok_packet(mysql, mysql->net.read_pos + 1, pkt_length)) return -1;

- if (!mysql->net.tls_self_signed_error) + if (!mysql->net.tls_verify_status || + ((mysql->net.tls_verify_status & MARIADB_TLS_VERIFY_TRUST) && + is_local_connection(mysql->net.pvio)))

Not needed, see above

return 0;

assert(mysql->options.use_ssl); diff --git a/libmariadb/secure/openssl.c b/libmariadb/secure/openssl.c index 8231a244..b5b573a9 100644 --- a/libmariadb/secure/openssl.c +++ b/libmariadb/secure/openssl.c @@ -459,24 +461,38 @@ error: return NULL; }

-unsigned int ma_tls_get_peer_cert_info(MARIADB_TLS *ctls) +unsigned int ma_tls_get_peer_cert_info(MARIADB_TLS *ctls, uint hash_size) { X509 *cert; + unsigned int hash_alg; SSL *ssl; + char fp[129];

+ switch (hash_size) { + case 0: + case 256: + hash_alg= MA_HASH_SHA256; + break; + case 384: + hash_alg= MA_HASH_SHA384; + break; + case 512: + hash_alg= MA_HASH_SHA512; + break; + default: + return 1; + } + if (!ctls || !ctls->ssl) return 1;

- /* Did we already read peer cert information ? */ - if (ctls->cert_info.version) - return 0; - ssl= (SSL *)ctls->ssl;

/* Store peer certificate information */ + if (!ctls->cert_info.version) + {

when can it be already set?

if ((cert= SSL_get_peer_certificate(ssl))) { - char fp[33]; #if OPENSSL_VERSION_NUMBER >= 0x10101000L const ASN1_TIME *not_before= X509_get0_notBefore(cert), *not_after= X509_get0_notAfter(cert); @@ -714,9 +714,40 @@ my_bool ma_tls_close(MARIADB_TLS *ctls) return rc; }

-int ma_tls_verify_server_cert(MARIADB_TLS *ctls) +/** Check for possible errors, and store the result in net.tls_verify_status. + verification will happen after handshake by ma_tls_verify_server_cert(). + To retrieve all errors, this callback function returns always true. + (By default OpenSSL stops verification after first error +*/ +static int ma_verification_callback(int preverify_ok __attribute__((unused)), X509_STORE_CTX *ctx) { - X509 *cert; + SSL *ssl; + + int x509_err= X509_STORE_CTX_get_error(ctx); + + if ((ssl = X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx()))) + { + MYSQL *mysql= (MYSQL *)SSL_get_app_data(ssl); + + if ((x509_err == X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT || + x509_err == X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN)) + mysql->net.tls_verify_status|= MARIADB_TLS_VERIFY_TRUST; + else if (x509_err == X509_V_ERR_CERT_REVOKED) + mysql->net.tls_verify_status|= MARIADB_TLS_VERIFY_REVOKED; + else if (x509_err == X509_V_ERR_CERT_NOT_YET_VALID || + x509_err == X509_V_ERR_CERT_HAS_EXPIRED) + mysql->net.tls_verify_status|= MARIADB_TLS_VERIFY_PERIOD; + else if (x509_err != X509_V_OK) + mysql->net.tls_verify_status|= MARIADB_TLS_VERIFY_UNKNOWN;

and here you need to store the error message in mysql->net. for example, else if (x509_err == X509_V_ERR_CERT_NOT_YET_VALID || x509_err == X509_V_ERR_CERT_HAS_EXPIRED) { if (mysql->net.tls_verify_status < MARIADB_TLS_VERIFY_PERIOD) mysql->net.tls_verify_error= X509_verify_cert_error_string(x509_err); mysql->net.tls_verify_status|= MARIADB_TLS_VERIFY_PERIOD; } or else if (x509_err == X509_V_ERR_CERT_NOT_YET_VALID || x509_err == X509_V_ERR_CERT_HAS_EXPIRED) { if (mysql->net.tls_verify_status < MARIADB_TLS_VERIFY_PERIOD) my_set_error(mysql, CR_SSL_CONNECTION_ERROR, SQLSTATE_UNKNOWN, ER(CR_SSL_CONNECTION_ERROR), X509_verify_cert_error_string(x509_err)); mysql->net.tls_verify_status|= MARIADB_TLS_VERIFY_PERIOD; }

+ } + + /* continue verification */ + return 1; +} + +int ma_tls_verify_server_cert(MARIADB_TLS *ctls, unsigned int verify_flags) +{ + X509 *cert= NULL; MYSQL *mysql; SSL *ssl; MARIADB_PVIO *pvio; @@ -734,52 +765,97 @@ int ma_tls_verify_server_cert(MARIADB_TLS *ctls) mysql= (MYSQL *)SSL_get_app_data(ssl); pvio= mysql->net.pvio;

+ if (verify_flags & MARIADB_TLS_VERIFY_FINGERPRINT) + { + if (ma_pvio_tls_check_fp(ctls, mysql->options.extension->tls_fp, mysql->options.extension->tls_fp_list)) + { + mysql->net.tls_verify_status |= MARIADB_TLS_VERIFY_FINGERPRINT; + return 1; + } + + /* if certificates are valid and no revocation error occured, + we can return */ + if (!(mysql->net.tls_verify_status & MARIADB_TLS_VERIFY_PERIOD) && + !(mysql->net.tls_verify_status & MARIADB_TLS_VERIFY_REVOKED))

you can check this before trying the fingerprint

+ { + mysql->net.tls_verify_status= MARIADB_TLS_VERIFY_OK; + return 0; + } + } + + if (mysql->net.tls_verify_status & verify_flags) + { + return 1; + } + + if (verify_flags & MARIADB_TLS_VERIFY_HOST) + { if (!mysql->host) { pvio->set_error(mysql, CR_SSL_CONNECTION_ERROR, SQLSTATE_UNKNOWN, ER(CR_SSL_CONNECTION_ERROR), "Invalid (empty) hostname"); - return 1; + mysql->net.tls_verify_status|= MARIADB_TLS_VERIFY_HOST; + return MARIADB_TLS_VERIFY_ERROR; }

if (!(cert= SSL_get_peer_certificate(ssl))) { pvio->set_error(mysql, CR_SSL_CONNECTION_ERROR, SQLSTATE_UNKNOWN, ER(CR_SSL_CONNECTION_ERROR), "Unable to get server certificate"); - return 1; + mysql->net.tls_verify_status|= MARIADB_TLS_VERIFY_HOST;

not sure it's MARIADB_TLS_VERIFY_HOST, may be MARIADB_TLS_VERIFY_UNKNOWN ?

+ return MARIADB_TLS_VERIFY_ERROR; } + #ifdef HAVE_OPENSSL_CHECK_HOST if (X509_check_host(cert, mysql->host, strlen(mysql->host), 0, 0) != 1 && X509_check_ip_asc(cert, mysql->host, 0) != 1) + { + mysql->net.tls_verify_status|= MARIADB_TLS_VERIFY_HOST; goto error; + } #else x509sn= X509_get_subject_name(cert);

if ((cn_pos= X509_NAME_get_index_by_NID(x509sn, NID_commonName, -1)) < 0) + { + mysql->net.tls_verify_status|= MARIADB_TLS_VERIFY_HOST; goto error; + }

if (!(cn_entry= X509_NAME_get_entry(x509sn, cn_pos))) + { + mysql->net.tls_verify_status|= MARIADB_TLS_VERIFY_HOST; goto error; + }

if (!(cn_asn1 = X509_NAME_ENTRY_get_data(cn_entry))) + { + mysql->net.tls_verify_status|= MARIADB_TLS_VERIFY_HOST; goto error; + }

cn_str = (char *)ASN1_STRING_data(cn_asn1);

/* Make sure there is no embedded \0 in the CN */ if ((size_t)ASN1_STRING_length(cn_asn1) != strlen(cn_str)) + { + mysql->net.tls_verify_status|= MARIADB_TLS_VERIFY_HOST; goto error; + }

if (strcmp(cn_str, mysql->host)) + { + mysql->net.tls_verify_status|= MARIADB_TLS_VERIFY_HOST; goto error; + } #endif X509_free(cert); - + } return 0; error: + if (cert) X509_free(cert);

- pvio->set_error(mysql, CR_SSL_CONNECTION_ERROR, SQLSTATE_UNKNOWN, - ER(CR_SSL_CONNECTION_ERROR), "Validation of SSL server certificate failed"); return 1; }

Regards, Sergei Chief Architect, MariaDB Server and security@mariadb.org