[glib/mcatanzaro/readable-private-key: 1/2] gtlscertificate: make private key properties readable

[Michael Catanzaro <mcatanzaro src gnome org>]

commit c50e543e9d08e9012f8614097d066286193a7147
Author: Michael Catanzaro <mcatanzaro gnome org>
Date:   Thu May 6 12:04:29 2021 -0500

    gtlscertificate: make private key properties readable
    
    WebKit wants these private key properties to be readable in order to
    implement a deserialization function. Currently they are read-only
    because at the time GTlsCertificate was originally designed, the plan
    was to support PKCS#11-backed private keys: private keys that are stored
    on a smartcard, where the private key is completely unreadable. The
    design goal was to support both memory-backed and smartcard-backed
    private keys with the same GTlsCertificate API, abstracting away the
    implementation differences such that code using GTlsCertificate doesn't
    need to know the difference.
    
    The original PKCS#11 implementation was never fully baked and at some
    point in the past I deleted it all. It has since been replaced with a
    new implementation, including a GTlsCertificate:private-key-pkcs11-uri
    property, which is readable. So our current API already exposes the
    differences between normal private keys and PKCS#11-backed private keys.
    The point of making the private-key and private-key-pem properties
    write-only was to avoid exposing this difference.
    
    Do we have to make this API function readable? No, because WebKit could
    be just as well served if we were to expose serialize and deserialize
    functions instead. But WebKit needs to support serializing and
    deserializing the non-private portion of GTlsCertificate with older
    versions of GLib anyway, so we can do whatever is nicest for GLib. And I
    think making this property readable is nicest, since the original design
    reason for it to not be readable is now obsolete. The disadvantage to
    this approach is that it's now possible for an application to read the
    private-key or private-key-pem property, receive NULL, and think "this
    certificate must not have a private key," which would be incorrect if
    the private-key-pkcs11-uri property is set. That seems like a minor
    risk, but it should be documented.

 gio/gtlscertificate.c       | 56 ++++++++++++++++++++++++-------------
 gio/tests/gtesttlsbackend.c |  6 ----
 gio/tests/gtesttlsbackend.h |  3 --
 gio/tests/tls-certificate.c | 67 ++++++++++++++++++++-------------------------
 4 files changed, 67 insertions(+), 65 deletions(-)
---
diff --git a/gio/gtlscertificate.c b/gio/gtlscertificate.c
index aadc562a6..cd5fd6403 100644
--- a/gio/gtlscertificate.c
+++ b/gio/gtlscertificate.c
@@ -84,6 +84,8 @@ g_tls_certificate_get_property (GObject    *object,
 {
   switch (prop_id)
     {
+    case PROP_PRIVATE_KEY:
+    case PROP_PRIVATE_KEY_PEM:
     case PROP_PKCS11_URI:
     case PROP_PRIVATE_KEY_PKCS11_URI:
       /* Subclasses must override this property but this allows older backends to not fatally error */
@@ -154,17 +156,25 @@ g_tls_certificate_class_init (GTlsCertificateClass *class)
                                                        G_PARAM_CONSTRUCT_ONLY |
                                                        G_PARAM_STATIC_STRINGS));
   /**
-   * GTlsCertificate:private-key:
+   * GTlsCertificate:private-key: (nullable)
    *
    * The DER (binary) encoded representation of the certificate's
-   * private key, in either PKCS#1 format or unencrypted PKCS#8
-   * format. This property (or the #GTlsCertificate:private-key-pem
-   * property) can be set when constructing a key (eg, from a file),
-   * but cannot be read.
+   * private key, in either [PKCS \#1 format](https://datatracker.ietf.org/doc/html/rfc8017)
+   * or unencrypted [PKCS \#8 format.](https://datatracker.ietf.org/doc/html/rfc5208)
+   * PKCS \#8 format is supported since 2.32; earlier releases only
+   * support PKCS \#1. You can use the `openssl rsa` tool to convert
+   * PKCS \#8 keys to PKCS \#1.
    *
-   * PKCS#8 format is supported since 2.32; earlier releases only
-   * support PKCS#1. You can use the `openssl rsa`
-   * tool to convert PKCS#8 keys to PKCS#1.
+   * This property (or the #GTlsCertificate:private-key-pem property)
+   * can be set when constructing a key (for example, from a file).
+   * Since GLib 2.70, it is now also readable; however, be aware that if
+   * the private key is backed by a PKCS \#11 URI – for example, if it
+   * is stored on a smartcard – then this property will be %NULL. If so,
+   * the private key must be referenced via its PKCS \#11 URI,
+   * #GTlsCertificate:private-key-pkcs11-uri. You must check both
+   * properties to see if the certificate really has a private key.
+   * When this property is read, the output format will be unencrypted
+   * PKCS \#8.
    *
    * Since: 2.28
    */
@@ -173,22 +183,30 @@ g_tls_certificate_class_init (GTlsCertificateClass *class)
                                                       P_("Private key"),
                                                       P_("The DER representation of the certificate’s 
private key"),
                                                       G_TYPE_BYTE_ARRAY,
-                                                      G_PARAM_WRITABLE |
+                                                      G_PARAM_READWRITE |
                                                       G_PARAM_CONSTRUCT_ONLY |
                                                       G_PARAM_STATIC_STRINGS));
   /**
-   * GTlsCertificate:private-key-pem:
+   * GTlsCertificate:private-key-pem: (nullable)
    *
    * The PEM (ASCII) encoded representation of the certificate's
-   * private key in either PKCS#1 format ("`BEGIN RSA PRIVATE
-   * KEY`") or unencrypted PKCS#8 format ("`BEGIN
-   * PRIVATE KEY`"). This property (or the
-   * #GTlsCertificate:private-key property) can be set when
-   * constructing a key (eg, from a file), but cannot be read.
+   * private key in either [PKCS \#1 format](https://datatracker.ietf.org/doc/html/rfc8017)
+   * ("`BEGIN RSA PRIVATE KEY`") or unencrypted
+   * [PKCS \#8 format](https://datatracker.ietf.org/doc/html/rfc5208)
+   * ("`BEGIN PRIVATE KEY`"). PKCS \#8 format is supported since 2.32;
+   * earlier releases only support PKCS \#1. You can use the `openssl rsa`
+   * tool to convert PKCS \#8 keys to PKCS \#1.
    *
-   * PKCS#8 format is supported since 2.32; earlier releases only
-   * support PKCS#1. You can use the `openssl rsa`
-   * tool to convert PKCS#8 keys to PKCS#1.
+   * This property (or the #GTlsCertificate:private-key property)
+   * can be set when constructing a key (for example, from a file).
+   * Since GLib 2.70, it is now also readable; however, be aware that if
+   * the private key is backed by a PKCS \#11 URI - for example, if it
+   * is stored on a smartcard - then this property will be %NULL. If so,
+   * the private key must be referenced via its PKCS \#11 URI,
+   * #GTlsCertificate:private-key-pkcs11-uri. You must check both
+   * properties to see if the certificate really has a private key.
+   * When this property is read, the output format will be unencrypted
+   * PKCS \#8.
    *
    * Since: 2.28
    */
@@ -197,7 +215,7 @@ g_tls_certificate_class_init (GTlsCertificateClass *class)
                                                        P_("Private key (PEM)"),
                                                        P_("The PEM representation of the certificate’s 
private key"),
                                                        NULL,
-                                                       G_PARAM_WRITABLE |
+                                                       G_PARAM_READWRITE |
                                                        G_PARAM_CONSTRUCT_ONLY |
                                                        G_PARAM_STATIC_STRINGS));
   /**
diff --git a/gio/tests/gtesttlsbackend.c b/gio/tests/gtesttlsbackend.c
index 1e07c1e5e..6c48c2394 100644
--- a/gio/tests/gtesttlsbackend.c
+++ b/gio/tests/gtesttlsbackend.c
@@ -407,12 +407,6 @@ g_test_tls_connection_initable_iface_init (GInitableIface  *iface)
   iface->init = g_test_tls_connection_initable_init;
 }
 
-const gchar *
-g_test_tls_connection_get_private_key_pem (GTlsCertificate *cert)
-{
-  return ((GTestTlsCertificate *)cert)->key_pem;
-}
-
 /* Test database type */
 
 typedef struct _GTestTlsDatabase      GTestTlsDatabase;
diff --git a/gio/tests/gtesttlsbackend.h b/gio/tests/gtesttlsbackend.h
index 11a8bf149..07948fddc 100644
--- a/gio/tests/gtesttlsbackend.h
+++ b/gio/tests/gtesttlsbackend.h
@@ -39,9 +39,6 @@ struct _GTestTlsBackendClass {
 
 GType _g_test_tls_backend_get_type       (void);
 
-const gchar *g_test_tls_connection_get_private_key_pem (GTlsCertificate *cert);
-
-
 G_END_DECLS
 
 #endif /* __G_TEST_TLS_BACKEND_H__ */
diff --git a/gio/tests/tls-certificate.c b/gio/tests/tls-certificate.c
index d0a908530..2995b1028 100644
--- a/gio/tests/tls-certificate.c
+++ b/gio/tests/tls-certificate.c
@@ -40,7 +40,7 @@ pem_parser (const Reference *ref)
   gchar *pem;
   gsize pem_len = 0;
   gchar *parsed_cert_pem = NULL;
-  const gchar *parsed_key_pem = NULL;
+  gchar *parsed_key_pem = NULL;
   GError *error = NULL;
 
   /* Check PEM parsing in certificate, private key order. */
@@ -55,13 +55,12 @@ pem_parser (const Reference *ref)
 
   g_object_get (cert,
       "certificate-pem", &parsed_cert_pem,
+      "private-key-pem", &parsed_key_pem,
       NULL);
-  parsed_key_pem = g_test_tls_connection_get_private_key_pem (cert);
   g_assert_cmpstr (parsed_cert_pem, ==, ref->cert_pems[0]);
-  g_free (parsed_cert_pem);
-  parsed_cert_pem = NULL;
+  g_clear_pointer (&parsed_cert_pem, g_free);
   g_assert_cmpstr (parsed_key_pem, ==, ref->key_pem);
-  parsed_key_pem = NULL;
+  g_clear_pointer (&parsed_key_pem, g_free);
 
   g_object_unref (cert);
 
@@ -89,13 +88,12 @@ pem_parser (const Reference *ref)
 
   g_object_get (cert,
       "certificate-pem", &parsed_cert_pem,
+      "private-key-pem", &parsed_key_pem,
       NULL);
-  parsed_key_pem = g_test_tls_connection_get_private_key_pem (cert);
   g_assert_cmpstr (parsed_cert_pem, ==, ref->cert_pems[0]);
-  g_free (parsed_cert_pem);
-  parsed_cert_pem = NULL;
+  g_clear_pointer (&parsed_cert_pem, g_free);
   g_assert_cmpstr (parsed_key_pem, ==, ref->key_pem);
-  parsed_key_pem = NULL;
+  g_clear_pointer (&parsed_key_pem, g_free);
 
   g_free (pem);
   g_object_unref (cert);
@@ -111,11 +109,10 @@ pem_parser (const Reference *ref)
 
   g_object_get (cert,
       "certificate-pem", &parsed_cert_pem,
+      "private-key-pem", &parsed_key_pem,
       NULL);
-  parsed_key_pem = g_test_tls_connection_get_private_key_pem (cert);
   g_assert_cmpstr (parsed_cert_pem, ==, ref->cert_pems[0]);
-  g_free (parsed_cert_pem);
-  parsed_cert_pem = NULL;
+  g_clear_pointer (&parsed_cert_pem, g_free);
   g_assert_null (parsed_key_pem);
 
   g_free (pem);
@@ -141,7 +138,7 @@ pem_parser_handles_chain (const Reference *ref)
   GTlsCertificate *original_cert;
   gchar *pem;
   gchar *parsed_cert_pem = NULL;
-  const gchar *parsed_key_pem = NULL;
+  gchar *parsed_key_pem = NULL;
   GError *error = NULL;
 
   /* Check that a chain with exactly three certificates is returned */
@@ -156,14 +153,14 @@ pem_parser_handles_chain (const Reference *ref)
 
   g_object_get (cert,
       "certificate-pem", &parsed_cert_pem,
+      "private-key-pem", &parsed_key_pem,
       NULL);
   g_assert_cmpstr (parsed_cert_pem, ==, ref->cert_pems[0]);
   g_clear_pointer (&parsed_cert_pem, g_free);
 
   /* Make sure the private key was parsed */
-  parsed_key_pem = g_test_tls_connection_get_private_key_pem (cert);
   g_assert_cmpstr (parsed_key_pem, ==, ref->key_pem);
-  parsed_key_pem = NULL;
+  g_clear_pointer (&parsed_key_pem, g_free);
 
   /* Now test the second cert */
   issuer = g_tls_certificate_get_issuer (cert);
@@ -175,12 +172,12 @@ pem_parser_handles_chain (const Reference *ref)
 
   g_object_get (cert,
       "certificate-pem", &parsed_cert_pem,
+      "private-key-pem", &parsed_key_pem,
       NULL);
   g_assert_cmpstr (parsed_cert_pem, ==, ref->cert_pems[1]);
   g_clear_pointer (&parsed_cert_pem, g_free);
 
   /* Only the first cert should have a private key */
-  parsed_key_pem = g_test_tls_connection_get_private_key_pem (cert);
   g_assert_null (parsed_key_pem);
 
   /* Now test the final cert */
@@ -190,11 +187,11 @@ pem_parser_handles_chain (const Reference *ref)
 
   g_object_get (cert,
       "certificate-pem", &parsed_cert_pem,
+      "private-key-pem", &parsed_key_pem,
       NULL);
   g_assert_cmpstr (parsed_cert_pem, ==, ref->cert_pems[2]);
   g_clear_pointer (&parsed_cert_pem, g_free);
 
-  parsed_key_pem = g_test_tls_connection_get_private_key_pem (cert);
   g_assert_null (parsed_key_pem);
 
   g_object_unref (original_cert);
@@ -237,7 +234,7 @@ from_file (const Reference *ref)
 {
   GTlsCertificate *cert;
   gchar *parsed_cert_pem = NULL;
-  const gchar *parsed_key_pem = NULL;
+  gchar *parsed_key_pem = NULL;
   GError *error = NULL;
 
   cert = g_tls_certificate_new_from_file (g_test_get_filename (G_TEST_DIST, "cert-tests", "key-cert.pem", 
NULL),
@@ -247,13 +244,12 @@ from_file (const Reference *ref)
 
   g_object_get (cert,
       "certificate-pem", &parsed_cert_pem,
+      "private-key-pem", &parsed_key_pem,
       NULL);
-  parsed_key_pem = g_test_tls_connection_get_private_key_pem (cert);
   g_assert_cmpstr (parsed_cert_pem, ==, ref->cert_pems[0]);
-  g_free (parsed_cert_pem);
-  parsed_cert_pem = NULL;
+  g_clear_pointer (&parsed_cert_pem, g_free);
   g_assert_cmpstr (parsed_key_pem, ==, ref->key_pem);
-  parsed_key_pem = NULL;
+  g_clear_pointer (&parsed_key_pem, g_free);
 
   g_object_unref (cert);
 }
@@ -263,7 +259,7 @@ from_files (const Reference *ref)
 {
   GTlsCertificate *cert;
   gchar *parsed_cert_pem = NULL;
-  const gchar *parsed_key_pem = NULL;
+  gchar *parsed_key_pem = NULL;
   GError *error = NULL;
 
   cert = g_tls_certificate_new_from_files (g_test_get_filename (G_TEST_DIST, "cert-tests", "cert1.pem", 
NULL),
@@ -274,13 +270,12 @@ from_files (const Reference *ref)
 
   g_object_get (cert,
       "certificate-pem", &parsed_cert_pem,
+      "private-key-pem", &parsed_key_pem,
       NULL);
-  parsed_key_pem = g_test_tls_connection_get_private_key_pem (cert);
   g_assert_cmpstr (parsed_cert_pem, ==, ref->cert_pems[0]);
-  g_free (parsed_cert_pem);
-  parsed_cert_pem = NULL;
+  g_clear_pointer (&parsed_cert_pem, g_free);
   g_assert_cmpstr (parsed_key_pem, ==, ref->key_pem);
-  parsed_key_pem = NULL;
+  g_clear_pointer (&parsed_key_pem, g_free);
 
   g_object_unref (cert);
 
@@ -332,7 +327,7 @@ from_files_crlf (const Reference *ref)
 {
   GTlsCertificate *cert;
   gchar *parsed_cert_pem = NULL;
-  const gchar *parsed_key_pem = NULL;
+  gchar *parsed_key_pem = NULL;
   GError *error = NULL;
 
   cert = g_tls_certificate_new_from_files (g_test_get_filename (G_TEST_DIST, "cert-tests", "cert-crlf.pem", 
NULL),
@@ -343,13 +338,12 @@ from_files_crlf (const Reference *ref)
 
   g_object_get (cert,
       "certificate-pem", &parsed_cert_pem,
+      "private-key-pem", &parsed_key_pem,
       NULL);
-  parsed_key_pem = g_test_tls_connection_get_private_key_pem (cert);
   g_assert_cmpstr (parsed_cert_pem, ==, ref->cert_crlf_pem);
-  g_free (parsed_cert_pem);
-  parsed_cert_pem = NULL;
+  g_clear_pointer (&parsed_cert_pem, g_free);
   g_assert_cmpstr (parsed_key_pem, ==, ref->key_crlf_pem);
-  parsed_key_pem = NULL;
+  g_clear_pointer (&parsed_key_pem, g_free);
 
   g_object_unref (cert);
 }
@@ -359,7 +353,7 @@ from_files_pkcs8 (const Reference *ref)
 {
   GTlsCertificate *cert;
   gchar *parsed_cert_pem = NULL;
-  const gchar *parsed_key_pem = NULL;
+  gchar *parsed_key_pem = NULL;
   GError *error = NULL;
 
   cert = g_tls_certificate_new_from_files (g_test_get_filename (G_TEST_DIST, "cert-tests", "cert1.pem", 
NULL),
@@ -370,13 +364,12 @@ from_files_pkcs8 (const Reference *ref)
 
   g_object_get (cert,
       "certificate-pem", &parsed_cert_pem,
+      "private-key-pem", &parsed_key_pem,
       NULL);
-  parsed_key_pem = g_test_tls_connection_get_private_key_pem (cert);
   g_assert_cmpstr (parsed_cert_pem, ==, ref->cert_pems[0]);
-  g_free (parsed_cert_pem);
-  parsed_cert_pem = NULL;
+  g_clear_pointer (&parsed_cert_pem, g_free);
   g_assert_cmpstr (parsed_key_pem, ==, ref->key8_pem);
-  parsed_key_pem = NULL;
+  g_clear_pointer (&parsed_key_pem, g_free);
 
   g_object_unref (cert);
 }