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 secur...@mariadb.org
_______________________________________________
developers mailing list -- developers@lists.mariadb.org
To unsubscribe send an email to developers-le...@lists.mariadb.org
