[glib/mcatanzaro/validation-flags: 1/3] Document potential footgun with GTlsCertificateFlags
- From: Michael Catanzaro <mcatanzaro src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [glib/mcatanzaro/validation-flags: 1/3] Document potential footgun with GTlsCertificateFlags
- Date: Wed, 4 Aug 2021 20:22:36 +0000 (UTC)
commit 90d40c8d161db5ce3f268198a2550caf05ec5251
Author: Michael Catanzaro <mcatanzaro redhat com>
Date: Wed Aug 4 14:57:05 2021 -0500
Document potential footgun with GTlsCertificateFlags
Once upon a time, we tried to return all possible certificate errors,
but it never actually worked reliably and nowadays we have given up.
This needs to be documented because a reasonable developer would not
expect it.
Because mistakes could be security-critical, I decided to copy the same
warning in several different places rather than relying only on
cross-referencese.
gio/gioenums.h | 14 ++++++++++----
gio/gtlscertificate.c | 9 +++++++--
gio/gtlsconnection.c | 10 ++++++++++
gio/gtlsdatabase.c | 22 +++++++++++++++-------
4 files changed, 42 insertions(+), 13 deletions(-)
---
diff --git a/gio/gioenums.h b/gio/gioenums.h
index d81ada416..953108369 100644
--- a/gio/gioenums.h
+++ b/gio/gioenums.h
@@ -1584,10 +1584,16 @@ typedef enum {
* flags
*
* A set of flags describing TLS certification validation. This can be
- * used to set which validation steps to perform (eg, with
- * g_tls_client_connection_set_validation_flags()), or to describe why
- * a particular certificate was rejected (eg, in
- * #GTlsConnection::accept-certificate).
+ * used to describe why a particular certificate was rejected (for
+ * example, in #GTlsConnection::accept-certificate).
+ *
+ * GLib guarantees that if certificate verification fails, at least one
+ * flag will be set, but it does not guarantee that all possible flags
+ * will be set. Accordingly, you may not safely decide to ignore any
+ * particular type of error. For example, it would be incorrect to mask
+ * %G_TLS_CERTIFICATE_EXPIRED if you want to allow expired certificates,
+ * because this could potentially be the only error flag set even if
+ * other problems exist with the certificate.
*
* Since: 2.28
*/
diff --git a/gio/gtlscertificate.c b/gio/gtlscertificate.c
index 308a0a7ed..100abc8c8 100644
--- a/gio/gtlscertificate.c
+++ b/gio/gtlscertificate.c
@@ -947,8 +947,13 @@ g_tls_certificate_get_issuer (GTlsCertificate *cert)
* @trusted_ca is %NULL, that bit will never be set in the return
* value.
*
- * (All other #GTlsCertificateFlags values will always be set or unset
- * as appropriate.)
+ * GLib guarantees that if certificate verification fails, at least one
+ * one error will be set in the return value, but it does not guarantee
+ * that all possible errors will be set. Accordingly, you may not safely
+ * decide to ignore any particular type of error. For example, it would
+ * beincorrect to mask %G_TLS_CERTIFICATE_EXPIRED if you want to allow
+ * expired certificates, because this could potentially be the only
+ * error flag set even if other problems exist with the certificate.
*
* Returns: the appropriate #GTlsCertificateFlags
*
diff --git a/gio/gtlsconnection.c b/gio/gtlsconnection.c
index 0239489b7..e04e0faf3 100644
--- a/gio/gtlsconnection.c
+++ b/gio/gtlsconnection.c
@@ -248,6 +248,14 @@ g_tls_connection_class_init (GTlsConnectionClass *klass)
* #GTlsConnection::accept-certificate overrode the default
* behavior.
*
+ * GLib guarantees that if certificate verification fails, at least
+ * one error will be set, but it does not guarantee that all possible
+ * errors will be set. Accordingly, you may not safely decide to
+ * ignore any particular type of error. For example, it would be
+ * incorrect to mask %G_TLS_CERTIFICATE_EXPIRED if you want to allow
+ * expired certificates, because this could potentially be the only
+ * error flag set even if other problems exist with the certificate.
+ *
* Since: 2.28
*/
g_object_class_install_property (gobject_class, PROP_PEER_CERTIFICATE_ERRORS,
@@ -655,6 +663,8 @@ g_tls_connection_get_peer_certificate (GTlsConnection *conn)
* certificate, after the handshake has completed or failed. (It is
* not set during the emission of #GTlsConnection::accept-certificate.)
*
+ * See #GTlsConnection:peer-certificate-errors for more information.
+ *
* Returns: @conn's peer's certificate errors
*
* Since: 2.28
diff --git a/gio/gtlsdatabase.c b/gio/gtlsdatabase.c
index d7dcf4bbe..044f19f36 100644
--- a/gio/gtlsdatabase.c
+++ b/gio/gtlsdatabase.c
@@ -489,13 +489,21 @@ g_tls_database_class_init (GTlsDatabaseClass *klass)
* used.
*
* If @chain is found to be valid, then the return value will be 0. If
- * @chain is found to be invalid, then the return value will indicate
- * the problems found. If the function is unable to determine whether
- * @chain is valid or not (eg, because @cancellable is triggered
- * before it completes) then the return value will be
- * %G_TLS_CERTIFICATE_GENERIC_ERROR and @error will be set
- * accordingly. @error is not set when @chain is successfully analyzed
- * but found to be invalid.
+ * @chain is found to be invalid, then the return value will indicate at
+ * least one problem found. If the function is unable to determine
+ * whether @chain is valid or not (for example, because @cancellable is
+ * triggered before it completes) then the return value will be
+ * %G_TLS_CERTIFICATE_GENERIC_ERROR and @error will be set accordingly.
+ * @error is not set when @chain is successfully analyzed but found to
+ * be invalid.
+ *
+ * GLib guarantees that if certificate verification fails, at least one
+ * one error will be set in the return value, but it does not guarantee
+ * that all possible errors will be set. Accordingly, you may not safely
+ * decide to ignore any particular type of error. For example, it would
+ * beincorrect to mask %G_TLS_CERTIFICATE_EXPIRED if you want to allow
+ * expired certificates, because this could potentially be the only
+ * error flag set even if other problems exist with the certificate.
*
* This function can block, use g_tls_database_verify_chain_async() to perform
* the verification operation asynchronously.
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]