[glib/mcatanzaro/validation-flags: 16/18] Document potential footgun with GTlsCertificateFlags




commit 780af9cff3cc636525a973c3f0c0244f2422a39e
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/gdtlsconnection.c | 17 +++++++++++++++++
 gio/gioenums.h        | 14 ++++++++++----
 gio/gtlscertificate.c |  9 +++++++--
 gio/gtlsconnection.c  | 19 +++++++++++++++++++
 gio/gtlsdatabase.c    | 22 +++++++++++++++-------
 5 files changed, 68 insertions(+), 13 deletions(-)
---
diff --git a/gio/gdtlsconnection.c b/gio/gdtlsconnection.c
index 880d87d2c..ec9234825 100644
--- a/gio/gdtlsconnection.c
+++ b/gio/gdtlsconnection.c
@@ -223,6 +223,14 @@ g_dtls_connection_default_init (GDtlsConnectionInterface *iface)
    * #GDtlsConnection::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.48
    */
   g_object_interface_install_property (iface,
@@ -314,6 +322,15 @@ g_dtls_connection_default_init (GDtlsConnectionInterface *iface)
    * signal handler. Otherwise, if no handler accepts the certificate,
    * the handshake will fail with %G_TLS_ERROR_BAD_CERTIFICATE.
    *
+   * GLib guarantees that if certificate verification fails, this signal
+   * will be emitted with at least one error will be set in @errors, 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 ignore
+   * %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.
+   *
    * For a server-side connection, @peer_cert is the certificate
    * presented by the client, if this was requested via the server's
    * #GDtlsServerConnection:authentication_mode. On the server side,
diff --git a/gio/gioenums.h b/gio/gioenums.h
index 2b6b5eb13..47ca06bed 100644
--- a/gio/gioenums.h
+++ b/gio/gioenums.h
@@ -1588,10 +1588,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 2c238120c..8231ea0e4 100644
--- a/gio/gtlscertificate.c
+++ b/gio/gtlscertificate.c
@@ -959,8 +959,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
+ * 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
+ * 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.
  *
  * Because TLS session context is not used, #GTlsCertificate may not
  * perform as many checks on the certificates as #GTlsConnection would.
diff --git a/gio/gtlsconnection.c b/gio/gtlsconnection.c
index 0239489b7..c4c305528 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,
@@ -339,6 +347,15 @@ g_tls_connection_class_init (GTlsConnectionClass *klass)
    * signal handler. Otherwise, if no handler accepts the certificate,
    * the handshake will fail with %G_TLS_ERROR_BAD_CERTIFICATE.
    *
+   * GLib guarantees that if certificate verification fails, this signal
+   * will be emitted with at least one error will be set in @errors, 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 ignore
+   * %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.
+   *
    * For a server-side connection, @peer_cert is the certificate
    * presented by the client, if this was requested via the server's
    * #GTlsServerConnection:authentication_mode. On the server side,
@@ -655,6 +672,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 2e5a264e9..13a0b6b68 100644
--- a/gio/gtlsdatabase.c
+++ b/gio/gtlsdatabase.c
@@ -485,13 +485,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 (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
+ * 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
+ * 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.
  *
  * Prior to GLib 2.48, GLib's default TLS backend modified @chain to
  * represent the certification path built by #GTlsDatabase during


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]